Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Do not return -EFAULT when the passed string has zero-length. Instead, > return -EINVAL when trying to create objects with empty names. Ok. You can commit this. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Gilles Chanteperdrix wrote:
Jan Kiszka wrote:
> Obviously a conversion error while switching to __xn_safe*.
>
> Signed-off-by: Jan Kiszka
Well, I have just checked the kernel code, and 0 as a return value of
strncpy_from_user is treated as a value in most places, even if not
-EFAULT.
>>> Better check our code: :) __xn_safe_strncpy_from_user works differently.
>> Then I would tend to consider that xn_safe_strncpy is broken.
> No, because it not a derivate of strncpy_from_user, but an internal
> service optimized for the most common use cases (where you don't care
> about the precise return value).
So, what should I call if I care about the return value ?
>>> The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see
>>> the issue: POSIX requires the length to report overflows to the users.
>>> Hmm, then back to the old code, just adding the missing address range check.
>>>
>> Forget that, now I should have looked into our code: The "safe" version
>> transparently calls __xn_strncpy_from_user, no need to go by foot here.
>>
>
> Here is the real bug:
>
>> long res;
>> __do_strncpy_from_user(dst, src, count, res);
>> 804505cc: 48 85 c9test %rcx,%rcx
>> 804505cf: 74 0e je 804505df
>>
>> 804505d1: ac lods %ds:(%rsi),%al
>> 804505d2: aa stos %al,%es:(%rdi)
>> 804505d3: 84 c0 test %al,%al
>> 804505d5: 74 05 je 804505dc
>>
>> 804505d7: 48 ff c9dec%rcx
>> 804505da: 75 f5 jne804505d1
>>
>> 804505dc: 48 29 c9sub%rcx,%rcx
> ^
>
>> return res;
>> }
>> 804505df: 48 89 c8mov%rcx,%rax
>> 804505e2: c9 leaveq
>> 804505e3: c3 retq
>
> And that is due to us lacking kernel commit
> e0a96129db574d6365e3439d16d88517c437ab33.
>
> Nevertheless, the existing checks against 0 in the POSIX layer weren't
> correct either. We need to fix both, but less ad-hoc than I tried.
IMO, sem_open(""), mq_open(""), shm_open("") should return an error. I
agree that -EFAULT is a bit harsh and that -EINVAL would be more
apropriate, but I do not want to remove that check.
--
Gilles.
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Obviously a conversion error while switching to __xn_safe*. Signed-off-by: Jan Kiszka >>> Well, I have just checked the kernel code, and 0 as a return value of >>> strncpy_from_user is treated as a value in most places, even if not >>> -EFAULT. >>> >> Better check our code: :) __xn_safe_strncpy_from_user works differently. > Then I would tend to consider that xn_safe_strncpy is broken. No, because it not a derivate of strncpy_from_user, but an internal service optimized for the most common use cases (where you don't care about the precise return value). >>> So, what should I call if I care about the return value ? >> The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see >> the issue: POSIX requires the length to report overflows to the users. >> Hmm, then back to the old code, just adding the missing address range check. >> > > Forget that, now I should have looked into our code: The "safe" version > transparently calls __xn_strncpy_from_user, no need to go by foot here. > Here is the real bug: > long res; > __do_strncpy_from_user(dst, src, count, res); > 804505cc: 48 85 c9test %rcx,%rcx > 804505cf: 74 0e je 804505df > > 804505d1: ac lods %ds:(%rsi),%al > 804505d2: aa stos %al,%es:(%rdi) > 804505d3: 84 c0 test %al,%al > 804505d5: 74 05 je 804505dc > > 804505d7: 48 ff c9dec%rcx > 804505da: 75 f5 jne804505d1 > > 804505dc: 48 29 c9sub%rcx,%rcx ^ > return res; > } > 804505df: 48 89 c8mov%rcx,%rax > 804505e2: c9 leaveq > 804505e3: c3 retq And that is due to us lacking kernel commit e0a96129db574d6365e3439d16d88517c437ab33. Nevertheless, the existing checks against 0 in the POSIX layer weren't correct either. We need to fix both, but less ad-hoc than I tried. Jan signature.asc Description: OpenPGP digital signature ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Obviously a conversion error while switching to __xn_safe*. >>> >>> Signed-off-by: Jan Kiszka >> Well, I have just checked the kernel code, and 0 as a return value of >> strncpy_from_user is treated as a value in most places, even if not >> -EFAULT. >> > Better check our code: :) __xn_safe_strncpy_from_user works differently. Then I would tend to consider that xn_safe_strncpy is broken. >>> No, because it not a derivate of strncpy_from_user, but an internal >>> service optimized for the most common use cases (where you don't care >>> about the precise return value). >> So, what should I call if I care about the return value ? > > The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see > the issue: POSIX requires the length to report overflows to the users. > Hmm, then back to the old code, just adding the missing address range check. > Forget that, now I should have looked into our code: The "safe" version transparently calls __xn_strncpy_from_user, no need to go by foot here. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Obviously a conversion error while switching to __xn_safe*. >> >> Signed-off-by: Jan Kiszka > Well, I have just checked the kernel code, and 0 as a return value of > strncpy_from_user is treated as a value in most places, even if not > -EFAULT. > Better check our code: :) __xn_safe_strncpy_from_user works differently. >>> Then I would tend to consider that xn_safe_strncpy is broken. >> No, because it not a derivate of strncpy_from_user, but an internal >> service optimized for the most common use cases (where you don't care >> about the precise return value). > > So, what should I call if I care about the return value ? The old combo of access_rok() and __xn_strncpy_from_user() - ah, I see the issue: POSIX requires the length to report overflows to the users. Hmm, then back to the old code, just adding the missing address range check. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Gilles Chanteperdrix wrote: Jan Kiszka wrote: > Obviously a conversion error while switching to __xn_safe*. > > Signed-off-by: Jan Kiszka Well, I have just checked the kernel code, and 0 as a return value of strncpy_from_user is treated as a value in most places, even if not -EFAULT. >>> Better check our code: :) __xn_safe_strncpy_from_user works differently. >> Then I would tend to consider that xn_safe_strncpy is broken. > > No, because it not a derivate of strncpy_from_user, but an internal > service optimized for the most common use cases (where you don't care > about the precise return value). So, what should I call if I care about the return value ? -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Gilles Chanteperdrix wrote: >>> Jan Kiszka wrote: Obviously a conversion error while switching to __xn_safe*. Signed-off-by: Jan Kiszka >>> Well, I have just checked the kernel code, and 0 as a return value of >>> strncpy_from_user is treated as a value in most places, even if not -EFAULT. >>> >> Better check our code: :) __xn_safe_strncpy_from_user works differently. > > Then I would tend to consider that xn_safe_strncpy is broken. No, because it not a derivate of strncpy_from_user, but an internal service optimized for the most common use cases (where you don't care about the precise return value). Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Gilles Chanteperdrix wrote: >> Jan Kiszka wrote: >>> Obviously a conversion error while switching to __xn_safe*. >>> >>> Signed-off-by: Jan Kiszka >> Well, I have just checked the kernel code, and 0 as a return value of >> strncpy_from_user is treated as a value in most places, even if not -EFAULT. >> > > Better check our code: :) __xn_safe_strncpy_from_user works differently. Then I would tend to consider that xn_safe_strncpy is broken. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Gilles Chanteperdrix wrote: > Jan Kiszka wrote: >> Obviously a conversion error while switching to __xn_safe*. >> >> Signed-off-by: Jan Kiszka > > Well, I have just checked the kernel code, and 0 as a return value of > strncpy_from_user is treated as a value in most places, even if not -EFAULT. > Better check our code: :) __xn_safe_strncpy_from_user works differently. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings
Jan Kiszka wrote: > Obviously a conversion error while switching to __xn_safe*. > > Signed-off-by: Jan Kiszka Well, I have just checked the kernel code, and 0 as a return value of strncpy_from_user is treated as a value in most places, even if not -EFAULT. -- Gilles. ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
