[Buildroot] [PATCH] package/libseccomp: circumvent uClibc-ng bug on x86_64

[Arnout Vandecappelle]

On 19/10/2019 03:15, unixmania at gmail.com wrote:
> From: Carlos Santos <unixmania at gmail.com>
> 
> On uClibc up to v1.0.31, syscall() for x86_64 is defined in
> libc/sysdeps/linux/x86_64/syscall.S as
> 
> syscall:
>         movq %rdi, %rax         /* Syscall number -> rax.  */
>         movq %rsi, %rdi         /* shift arg1 - arg5.  */
>         movq %rdx, %rsi
>         movq %rcx, %rdx
>         movq %r8, %r10
>         movq %r9, %r8
>         movq 8(%rsp),%r9        /* arg6 is on the stack.  */
>         syscall                 /* Do the system call.  */
>         cmpq $-4095, %rax       /* Check %rax for error.  */
>         jae __syscall_error     /* Branch forward if it failed.  */
>         ret                     /* Return to caller.  */
> 
> And __syscall_error is defined in
> libc/sysdeps/linux/x86_64/__syscall_error.c as
> 
> int __syscall_error(void) attribute_hidden;
> int __syscall_error(void)
> {
>         register int err_no __asm__ ("%rcx");
>         __asm__ ("mov %rax, %rcx\n\t"
>                  "neg %rcx");
>         __set_errno(err_no);
>         return -1;
> }
> 
> Notice that __syscall_error returns -1 as a 32-bit int in %rax, a 64-bit
> register i.e. 0x00000000ffffffff (decimal 4294967295). When this value
> is compared to -1 in _sys_chk_seccomp_flag_kernel() the result is false,
> leading the function to always return 0.
> 
> Prevent the error by coercing the return value of syscall() to int in a
> temporary variable before comparing it to -1. We could use just an (int)
> cast but the variable makes the code more readable

 Actually, I disagree with that. The intermediate variable hides the cast, and
the cast is the essential thing here. So if I were upstream, I'd want the
explicit cast, *and* a big fat comment above it to explain why it is needed.

 For us, however, that reasoning doesn't really fly because we see the full
patch, not the source code.

> and the machine code
> generated by the compiler is the same in both cases.
> 
> All other syscall() invocations were inspected and they either already
> coerce the result to int or do not compare it to -1.
> 
> The same problem probably occurs on other 64-bit systems but so far only
> x86_64 was tested.
> 
> A bug report is being submitted to uClibc.
> 
> Upstream status: https://github.com/seccomp/libseccomp/pull/175
> 
> Signed-off-by: Carlos Santos <unixmania at gmail.com>

 I updated both the commit message and the patch to refer to uClibc 1.0.32
instead and applied to master, thanks.

 Regards,
 Arnout