diff mbox series

package/libseccomp: circumvent uClibc-ng bug on x86_64

Message ID 20191019011525.10572-1-unixmania@gmail.com
State Not Applicable, archived
Headers show
Series package/libseccomp: circumvent uClibc-ng bug on x86_64 | expand

Commit Message

Carlos Santos Oct. 19, 2019, 1:15 a.m. UTC 
From: Carlos Santos <unixmania@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 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@gmail.com>
---
 ...n-uClibc-ng-syscall-on-x86_64-system.patch | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 package/libseccomp/0002-Circumvent-bug-in-uClibc-ng-syscall-on-x86_64-system.patch

Comments

Peter Korsgaard Oct. 30, 2019, 9:17 a.m. UTC | #1 
>>>>> "unixmania" == unixmania  <unixmania@gmail.com> writes:

 > From: Carlos Santos <unixmania@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 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@gmail.com>

Committed to 2019.02.x and 2019.08.x, thanks.
diff mbox series

Patch

diff --git a/package/libseccomp/0002-Circumvent-bug-in-uClibc-ng-syscall-on-x86_64-system.patch b/package/libseccomp/0002-Circumvent-bug-in-uClibc-ng-syscall-on-x86_64-system.patch
new file mode 100644
index 0000000000..a7151d840f
--- /dev/null
+++ b/package/libseccomp/0002-Circumvent-bug-in-uClibc-ng-syscall-on-x86_64-system.patch
@@ -0,0 +1,80 @@ 
+From 613e601bb4b50dc359b41f162a5b629449e4bbea Mon Sep 17 00:00:00 2001
+From: Carlos Santos <casantos@redhat.com>
+Date: Fri, 18 Oct 2019 22:02:49 -0300
+Subject: [PATCH] Circumvent bug in uClibc-ng syscall() on x86_64 systems
+
+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 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.
+
+Signed-off-by: Carlos Santos <casantos@redhat.com>
+---
+ src/system.c | 8 +++++---
+ 1 file changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/src/system.c b/src/system.c
+index 8e5aafc..811b401 100644
+--- a/src/system.c
++++ b/src/system.c
+@@ -215,10 +215,12 @@ static int _sys_chk_seccomp_flag_kernel(int flag)
+ 	/* this is an invalid seccomp(2) call because the last argument
+ 	 * is NULL, but depending on the errno value of EFAULT we can
+ 	 * guess if the filter flag is supported or not */
+-	if (sys_chk_seccomp_syscall() == 1 &&
+-	    syscall(_nr_seccomp, SECCOMP_SET_MODE_FILTER, flag, NULL) == -1 &&
+-	    errno == EFAULT)
++	int rc;
++	if (sys_chk_seccomp_syscall() == 1) {
++	    rc = syscall(_nr_seccomp, SECCOMP_SET_MODE_FILTER, flag, NULL);
++	    if (rc == -1 && errno == EFAULT)
+ 		return 1;
++	}
+ 
+ 	return 0;
+ }
+-- 
+2.18.1
+