Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> [ Please pull from git://git.xenomai.org/xenomai-jki.git for-2.4.x ]
>>
>> The test for __xn_access_ok was inverted, thus rejected valid requests.
>> Fix this and refactor the code in order to check for the actual size of
>> the passed fd sets.
>>
>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
>> ---
>>
>>  ksrc/skins/posix/syscall.c |   21 +++++++++------------
>>  1 files changed, 9 insertions(+), 12 deletions(-)
>>
>> diff --git a/ksrc/skins/posix/syscall.c b/ksrc/skins/posix/syscall.c
>> index 5c62a45..f0111f8 100644
>> --- a/ksrc/skins/posix/syscall.c
>> +++ b/ksrc/skins/posix/syscall.c
>> @@ -2384,17 +2384,16 @@ static int __select(struct task_struct *curr, struct 
>> pt_regs *regs)
>>      struct timeval tv;
>>      pthread_t thread;
>>      int i, err, nfds;
>> +    size_t fds_size;
>>  
>>      thread = pse51_current_thread();
>>      if (!thread)
>>              return -EPERM;
>>  
>>      if (__xn_reg_arg5(regs)) {
>> -            if (__xn_access_ok(curr, VERIFY_WRITE,
>> -                               __xn_reg_arg5(regs), sizeof(tv)))
>> -                    return -EFAULT;
>> -
>> -            if (__xn_copy_from_user(curr, &tv,
>> +            if (!__xn_access_ok(curr, VERIFY_WRITE,
>> +                                __xn_reg_arg5(regs), sizeof(tv)) ||
>> +                __xn_copy_from_user(curr, &tv,
>>                                      (void __user *)__xn_reg_arg5(regs),
>>                                      sizeof(tv)))
>>                      return -EFAULT;
> 
> Ok.
> 
>> @@ -2407,19 +2406,17 @@ static int __select(struct task_struct *curr, struct 
>> pt_regs *regs)
>>      }
>>  
>>      nfds = __xn_reg_arg1(regs);
>> +    fds_size = __FDELT(nfds + __NFDBITS - 1) * sizeof(long);
>>  
>>      for (i = 0; i < XNSELECT_MAX_TYPES; i++)
>>              if (ufd_sets[i]) {
>>                      in_fds[i] = &in_fds_storage[i];
>>                      out_fds[i] = & out_fds_storage[i];
>> -                    if (__xn_access_ok(curr, VERIFY_WRITE,
>> -                                       ufd_sets[i], sizeof(fd_set)))
>> -                            return -EFAULT;
>> -
>> -                    if (__xn_copy_from_user(curr, in_fds[i],
>> +                    if (!__xn_access_ok(curr, VERIFY_WRITE,
>> +                                        ufd_sets[i], fds_size) ||
> 
> I do not think this is a good idea. There is no special mention to it in
> select specification, but at least select prototype suggests that the
> user-space structure should be a valid fd_set, not a partial subset of it.

Yeah, and we copy a full set back. Missed that, will update both patches.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to