Message ID | 20200815213138.1375595-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | x32: Add <asm/unistd_x32.h> and regenerate arch-syscall.h | expand |
* H. J. Lu via Libc-alpha: > diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h > new file mode 100644 > index 0000000000..4de22a7737 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h > @@ -0,0 +1,9 @@ > +/* FIXME: __NR_set_thread_area should come from the kernel header file. > + This file should be removed if <asm/unistd_x32.h> from the kernel > + header file contains __NR_set_thread_area. */ > + > +#include_next <asm/unistd_x32.h> > + > +#ifndef __NR_set_thread_area > +# define __NR_set_thread_area 1073742029 > +#endif I think you can use sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h for this. It's more tailored to this purpose. arc and arm already use it. Thanks, Florian
On Mon, Aug 24, 2020 at 4:17 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu via Libc-alpha: > > > diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h > > new file mode 100644 > > index 0000000000..4de22a7737 > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h > > @@ -0,0 +1,9 @@ > > +/* FIXME: __NR_set_thread_area should come from the kernel header file. > > + This file should be removed if <asm/unistd_x32.h> from the kernel > > + header file contains __NR_set_thread_area. */ > > + > > +#include_next <asm/unistd_x32.h> > > + > > +#ifndef __NR_set_thread_area > > +# define __NR_set_thread_area 1073742029 > > +#endif > > I think you can use sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > for this. It's more tailored to this purpose. arc and arm already > use it. It doesn't work in this case. <fixup-asm-unistd.h> is used to undefine or redefine a syscall from <asm/unistd.h>. The actual syscall numbers still come from <asm/unistd.h>. In this case, the sycall number is missing from <asm/unistd_x32.h>. My patch is a prerequisite for a new testcase: ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c: In function ‘setup_ldt’: ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c:258:14: error: ‘SYS_set_thread_area’ undeclared (first use in this function) 258 | if (syscall(SYS_set_thread_area, &gdt_data16_desc) == 0) { | ^~~~~~~~~~~~~~~~~~~
* H. J. Lu: > It doesn't work in this case. <fixup-asm-unistd.h> is used to undefine or > redefine a syscall from <asm/unistd.h>. The actual syscall numbers still > come from <asm/unistd.h>. In this case, the sycall number is missing from > <asm/unistd_x32.h>. My patch is a prerequisite for a new testcase: > > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c: In function ‘setup_ldt’: > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c:258:14: error: > ‘SYS_set_thread_area’ undeclared (first use in this function) > 258 | if (syscall(SYS_set_thread_area, &gdt_data16_desc) == 0) { > | ^~~~~~~~~~~~~~~~~~~ Can you make it an internal test and use __NR_set_thread_area? Thanks, Florian
On Mon, Aug 24, 2020 at 6:23 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > It doesn't work in this case. <fixup-asm-unistd.h> is used to undefine or > > redefine a syscall from <asm/unistd.h>. The actual syscall numbers still > > come from <asm/unistd.h>. In this case, the sycall number is missing from > > <asm/unistd_x32.h>. My patch is a prerequisite for a new testcase: > > > > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c: In function ‘setup_ldt’: > > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c:258:14: error: > > ‘SYS_set_thread_area’ undeclared (first use in this function) > > 258 | if (syscall(SYS_set_thread_area, &gdt_data16_desc) == 0) { > > | ^~~~~~~~~~~~~~~~~~~ > > Can you make it an internal test and use __NR_set_thread_area? > Like? #if !defined(__NR_set_thread_area) && defined(__x86_64__) && defined(__ILP32__) /* X32 uses the same 64-bit syscall interface for set_thread_area. */ # define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) #endif
* H. J. Lu: > On Mon, Aug 24, 2020 at 6:23 AM Florian Weimer <fweimer@redhat.com> wrote: >> >> * H. J. Lu: >> >> > It doesn't work in this case. <fixup-asm-unistd.h> is used to undefine or >> > redefine a syscall from <asm/unistd.h>. The actual syscall numbers still >> > come from <asm/unistd.h>. In this case, the sycall number is missing from >> > <asm/unistd_x32.h>. My patch is a prerequisite for a new testcase: >> > >> > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c: In function ‘setup_ldt’: >> > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c:258:14: error: >> > ‘SYS_set_thread_area’ undeclared (first use in this function) >> > 258 | if (syscall(SYS_set_thread_area, &gdt_data16_desc) == 0) { >> > | ^~~~~~~~~~~~~~~~~~~ >> >> Can you make it an internal test and use __NR_set_thread_area? >> > > Like? > > #if !defined(__NR_set_thread_area) && defined(__x86_64__) && defined(__ILP32__) > /* X32 uses the same 64-bit syscall interface for set_thread_area. */ > # define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) > #endif No, I neant using tests-internal, so that you get the built-in system call table. But is it really correct to add an unsupported system call definition for the rest of glibc? If this is just setting things up for a test, maybe the __X32_SYSCALL_BIT hack is the right approach after all. (I don't know the objective of this test.) Thanks, Florian
On Mon, Aug 24, 2020 at 10:17 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > On Mon, Aug 24, 2020 at 6:23 AM Florian Weimer <fweimer@redhat.com> wrote: > >> > >> * H. J. Lu: > >> > >> > It doesn't work in this case. <fixup-asm-unistd.h> is used to undefine or > >> > redefine a syscall from <asm/unistd.h>. The actual syscall numbers still > >> > come from <asm/unistd.h>. In this case, the sycall number is missing from > >> > <asm/unistd_x32.h>. My patch is a prerequisite for a new testcase: > >> > > >> > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c: In function ‘setup_ldt’: > >> > ../sysdeps/unix/sysv/linux/x86/tst-sigreturn-1.c:258:14: error: > >> > ‘SYS_set_thread_area’ undeclared (first use in this function) > >> > 258 | if (syscall(SYS_set_thread_area, &gdt_data16_desc) == 0) { > >> > | ^~~~~~~~~~~~~~~~~~~ > >> > >> Can you make it an internal test and use __NR_set_thread_area? > >> > > > > Like? > > > > #if !defined(__NR_set_thread_area) && defined(__x86_64__) && defined(__ILP32__) > > /* X32 uses the same 64-bit syscall interface for set_thread_area. */ > > # define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) > > #endif > > No, I neant using tests-internal, so that you get the built-in system > call table. Since __NR_set_thread_area may not be defined in the kernel header file, move the test to tests-internal doesn't solve the problem. > But is it really correct to add an unsupported system call definition > for the rest of glibc? If this is just setting things up for a test, > maybe the __X32_SYSCALL_BIT hack is the right approach after all. > (I don't know the objective of this test.) set_thread_area is supported on x32. The only thing missing is #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) in <asm/unistd_x32.h>. Should I update my original patch to #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205)
* H. J. Lu: > Since __NR_set_thread_area may not be defined in the kernel header file, > move the test to tests-internal doesn't solve the problem. I expect that internal tests use the built-in system call tables, so <fixup-asm-unistd.h> should be effective for them (indirectly, after regenerating the tables). >> But is it really correct to add an unsupported system call definition >> for the rest of glibc? If this is just setting things up for a test, >> maybe the __X32_SYSCALL_BIT hack is the right approach after all. >> (I don't know the objective of this test.) > > set_thread_area is supported on x32. The only thing missing is > > #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) > > in <asm/unistd_x32.h>. > > Should I update my original patch to > > #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) But set_thread_area is not supported for application use, so it's an internal/whitebox test no matter what. What exactly are you trying to test? Thanks, Florian
On Mon, Aug 24, 2020 at 12:10 PM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > Since __NR_set_thread_area may not be defined in the kernel header file, > > move the test to tests-internal doesn't solve the problem. > > I expect that internal tests use the built-in system call tables, so > <fixup-asm-unistd.h> should be effective for them (indirectly, after > regenerating the tables). <fixup-asm-unistd.h> can't be used to add a syscal number. > >> But is it really correct to add an unsupported system call definition > >> for the rest of glibc? If this is just setting things up for a test, > >> maybe the __X32_SYSCALL_BIT hack is the right approach after all. > >> (I don't know the objective of this test.) > > > > set_thread_area is supported on x32. The only thing missing is > > > > #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) > > > > in <asm/unistd_x32.h>. > > > > Should I update my original patch to > > > > #define __NR_set_thread_area (__X32_SYSCALL_BIT + 205) > > But set_thread_area is not supported for application use, so it's an > internal/whitebox test no matter what. True. > What exactly are you trying to test? I am putting tools/testing/selftests/x86/sigreturn.c from Linux kernel into glibc to test a new arch_prctl syscall for CET: ARCH_X86_CET_MMAP_SHSTK: /* Allocate a new shadow stack with unsigned long long *addr: IN: requested shadow stack size: addr[0]. IN: The mmap flags: addr[1]. 1. MAP_32BIT 2. MAP_POPULATE OUT: allocated shadow stack address: *addr. */
* H. J. Lu: > I am putting tools/testing/selftests/x86/sigreturn.c from Linux kernel > into glibc to test a new arch_prctl syscall for CET: > > ARCH_X86_CET_MMAP_SHSTK: > > /* Allocate a new shadow stack with unsigned long long *addr: > IN: requested shadow stack size: addr[0]. > IN: The mmap flags: addr[1]. > 1. MAP_32BIT > 2. MAP_POPULATE > OUT: allocated shadow stack address: *addr. > */ This patch works for me. Thanks, Flroian diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile index 16b768d8ba..2cd35c120b 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile @@ -3,6 +3,8 @@ default-abi := x32 ifeq ($(subdir),misc) sysdep_routines += arch_prctl + +tests-internal += tst-set_thread_area endif ifeq ($(subdir),conform) diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h index 1a701c1472..7b64b1fa0c 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h @@ -260,6 +260,7 @@ #define __NR_sendto 1073741868 #define __NR_set_mempolicy 1073742062 #define __NR_set_robust_list 1073742354 +#define __NR_set_thread_area 1073742029 #define __NR_set_tid_address 1073742042 #define __NR_setdomainname 1073741995 #define __NR_setfsgid 1073741947 diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h new file mode 100644 index 0000000000..984f948626 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h @@ -0,0 +1,3 @@ +#ifndef __NR_set_thread_area +# define __NR_set_thread_area 1073742029 +#endif diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c b/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c new file mode 100644 index 0000000000..ff2249d343 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c @@ -0,0 +1,9 @@ +#include <sys/syscall.h> + +static int +do_test (void) +{ + return __NR_set_thread_area != 0; +} + +#include <support/test-driver.c>
On Tue, Aug 25, 2020 at 3:50 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu: > > > I am putting tools/testing/selftests/x86/sigreturn.c from Linux kernel > > into glibc to test a new arch_prctl syscall for CET: > > > > ARCH_X86_CET_MMAP_SHSTK: > > > > /* Allocate a new shadow stack with unsigned long long *addr: > > IN: requested shadow stack size: addr[0]. > > IN: The mmap flags: addr[1]. > > 1. MAP_32BIT > > 2. MAP_POPULATE > > OUT: allocated shadow stack address: *addr. > > */ > > This patch works for me. > > Thanks, > Flroian > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > index 16b768d8ba..2cd35c120b 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > @@ -3,6 +3,8 @@ default-abi := x32 > > ifeq ($(subdir),misc) > sysdep_routines += arch_prctl > + > +tests-internal += tst-set_thread_area > endif > > ifeq ($(subdir),conform) > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h > index 1a701c1472..7b64b1fa0c 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h > @@ -260,6 +260,7 @@ > #define __NR_sendto 1073741868 > #define __NR_set_mempolicy 1073742062 > #define __NR_set_robust_list 1073742354 > +#define __NR_set_thread_area 1073742029 > #define __NR_set_tid_address 1073742042 > #define __NR_setdomainname 1073741995 > #define __NR_setfsgid 1073741947 > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > new file mode 100644 > index 0000000000..984f948626 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > @@ -0,0 +1,3 @@ > +#ifndef __NR_set_thread_area > +# define __NR_set_thread_area 1073742029 > +#endif > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c b/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c > new file mode 100644 > index 0000000000..ff2249d343 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/tst-set_thread_area.c > @@ -0,0 +1,9 @@ > +#include <sys/syscall.h> > + > +static int > +do_test (void) > +{ > + return __NR_set_thread_area != 0; > +} > + > +#include <support/test-driver.c> > Here is the updated patch. OK for master? Thanks.
* H. J. Lu via Libc-alpha: > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > index 16b768d8ba..2cd35c120b 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > @@ -3,6 +3,8 @@ default-abi := x32 > > ifeq ($(subdir),misc) > sysdep_routines += arch_prctl > + > +tests-internal += tst-set_thread_area > endif I do not think the test is valuable. I just added it to show that things do actually compile with older Linux headers because you didn't believe me. 8-) > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > new file mode 100644 > index 0000000000..01d1f448ca > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > +/* X32 uses the same 64-bit syscall interface for set_thread_area. */ > +#ifndef __NR_set_thread_area > +# define __NR_set_thread_area 1073742029 > +#endif I think the comment should reference the Linux version in which the missing system call was fixed. Thanks, Florian
On Wed, Aug 26, 2020 at 2:35 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu via Libc-alpha: > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > > index 16b768d8ba..2cd35c120b 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile > > @@ -3,6 +3,8 @@ default-abi := x32 > > > > ifeq ($(subdir),misc) > > sysdep_routines += arch_prctl > > + > > +tests-internal += tst-set_thread_area > > endif > > I do not think the test is valuable. I just added it to show that > things do actually compile with older Linux headers because you didn't > believe me. 8-) Removed. > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > > new file mode 100644 > > index 0000000000..01d1f448ca > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/fixup-asm-unistd.h > > > +/* X32 uses the same 64-bit syscall interface for set_thread_area. */ > > +#ifndef __NR_set_thread_area > > +# define __NR_set_thread_area 1073742029 > > +#endif > > I think the comment should reference the Linux version in which the > missing system call was fixed. > Given that set_thread_area isn't for user application, I don't know when my fix will be merged into the upstream kernel. Here is the updated patch. OK for master?
* H. J. Lu: > Given that set_thread_area isn't for user application, I don't know when > my fix will be merged into the upstream kernel. > > Here is the updated patch. OK for master? Yes, this looks okay to me. Thanks, Florian
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index 37b844f839bc..670193a73f6e 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -213,7 +213,7 @@ 202 common futex sys_futex 203 common sched_setaffinity sys_sched_setaffinity 204 common sched_getaffinity sys_sched_getaffinity -205 64 set_thread_area +205 common set_thread_area 206 64 io_setup sys_io_setup 207 common io_destroy sys_io_destroy 208 common io_getevents sys_io_getevents -- 2.26.2 Add <asm/unistd_x32.h> for older kernel header files and regenerate x32 arch-syscall.h. --- sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h | 9 +++++++++ sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h | 1 + 2 files changed, 10 insertions(+) create mode 100644 sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h new file mode 100644 index 0000000000..4de22a7737 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/include/asm/unistd_x32.h @@ -0,0 +1,9 @@ +/* FIXME: __NR_set_thread_area should come from the kernel header file. + This file should be removed if <asm/unistd_x32.h> from the kernel + header file contains __NR_set_thread_area. */ + +#include_next <asm/unistd_x32.h> + +#ifndef __NR_set_thread_area +# define __NR_set_thread_area 1073742029 +#endif diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h index 1a701c1472..7b64b1fa0c 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h @@ -260,6 +260,7 @@ #define __NR_sendto 1073741868 #define __NR_set_mempolicy 1073742062 #define __NR_set_robust_list 1073742354 +#define __NR_set_thread_area 1073742029 #define __NR_set_tid_address 1073742042 #define __NR_setdomainname 1073741995 #define __NR_setfsgid 1073741947