Re: [Xenomai-core] [PATCH] posix: Fix error checks when copying user strings

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 2681 bytes --]

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 <jan.kiszka@domain.hid>
>>>>>>> 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);
> ffffffff804505cc:       48 85 c9                test   %rcx,%rcx
> ffffffff804505cf:       74 0e                   je     ffffffff804505df <rthal_strncpy_from_user+0x1f>
> ffffffff804505d1:       ac                      lods   %ds:(%rsi),%al
> ffffffff804505d2:       aa                      stos   %al,%es:(%rdi)
> ffffffff804505d3:       84 c0                   test   %al,%al
> ffffffff804505d5:       74 05                   je     ffffffff804505dc <rthal_strncpy_from_user+0x1c>
> ffffffff804505d7:       48 ff c9                dec    %rcx
> ffffffff804505da:       75 f5                   jne    ffffffff804505d1 <rthal_strncpy_from_user+0x11>
> ffffffff804505dc:       48 29 c9                sub    %rcx,%rcx
                                                         ^^^^^^^^^

>         return res;
> }
> ffffffff804505df:       48 89 c8                mov    %rcx,%rax
> ffffffff804505e2:       c9                      leaveq
> ffffffff804505e3:       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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]