* [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810]
@ 2020-04-13 14:25 H.J. Lu
2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw)
To: libc-alpha
X32 has 32-bit long and pointer with 64-bit off_t. Since x32 psABI
requires that pointers passed in registers must be zero-extended to
64bit, x32 can share many syscall interfaces with LP64. When a LP64
syscall with long and unsigned long arguments is used for x32, these
arguments must be properly extended to 64-bit. Otherwise if the upper
32 bits of the register have undefined value, such a syscall will be
rejected by kernel.
For syscalls implemented in assembly codes, 'U' is added to syscall
signature key letters for unsigned long. SYSCALL_ULONG_ARG_1 and
SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first
and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is
defined. They are used by x32 to zero-extend 32-bit arguments to
64 bits.
For x32 INLINE_SYSCALLs, cast
1. pointer to unsigned long (32 bit).
2. 32-bit unsigned integer to unsigned long long (64 bit).
3. 32-bit integer to long long (64 bit).
For
void *mmap(void *addr, size_t length, int prot, int flags,
int fd, off_t offset);
we now generate
0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d
7: 75 1f jne 28 <__mmap64+0x28>
9: 48 63 d2 movslq %edx,%rdx
c: 89 f6 mov %esi,%esi
e: 4d 63 c0 movslq %r8d,%r8
11: 4c 63 d1 movslq %ecx,%r10
14: b8 09 00 00 40 mov $0x40000009,%eax
19: 0f 05 syscall
That is
1. addr is unchanged.
2. length is zero-extend to 64 bits.
3. prot is sign-extend to 64 bits.
4. flags is sign-extend to 64 bits.
5. fd is sign-extend to 64 bits.
6. offset is unchanged.
For int arguments, since kernel uses only the lower 32 bits and ignores
the upper 32 bits in 64-bit registers, these work correctly.
Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
H.J. Lu (3):
Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
x32: Properly pass long to syscall [BZ #25810]
Add a syscall test for [BZ #25810]
misc/Makefile | 2 +-
misc/tst-syscalls.c | 146 ++++++++++++++++++++
sysdeps/unix/make-syscalls.sh | 88 ++++++++++++
sysdeps/unix/syscall-template.S | 43 +++++-
sysdeps/unix/syscalls.list | 6 +-
sysdeps/unix/sysv/linux/syscalls.list | 14 +-
sysdeps/unix/sysv/linux/x86_64/sysdep.h | 80 ++++++++---
sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 38 +++++
8 files changed, 385 insertions(+), 32 deletions(-)
create mode 100644 misc/tst-syscalls.c
--
2.25.2
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810] 2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu @ 2020-04-13 14:25 ` H.J. Lu 2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu 2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu 2 siblings, 0 replies; 13+ messages in thread From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw) To: libc-alpha X32 has 32-bit long and pointer with 64-bit off_t. Since x32 psABI requires that pointers passed in registers must be zero-extended to 64bit, x32 can share many syscall interfaces with LP64. When a LP64 syscall with long and unsigned long arguments is used for x32, these arguments must be properly extended to 64-bit. Otherwise if the upper 32 bits of the register have undefined value, such a syscall will be rejected by kernel. For syscalls implemented in assembly codes, 'U' is added to syscall signature key letters for unsigned long. SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is defined. They are used by x32 to zero-extend 32-bit arguments to 64 bits. Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py. --- sysdeps/unix/make-syscalls.sh | 88 +++++++++++++++++++++ sysdeps/unix/syscall-template.S | 43 +++++++++- sysdeps/unix/syscalls.list | 6 +- sysdeps/unix/sysv/linux/syscalls.list | 14 ++-- sysdeps/unix/sysv/linux/x86_64/sysdep.h | 70 ++++++++++++---- sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 20 +++++ 6 files changed, 214 insertions(+), 27 deletions(-) diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh index c07626677f..4a28badea6 100644 --- a/sysdeps/unix/make-syscalls.sh +++ b/sysdeps/unix/make-syscalls.sh @@ -30,6 +30,7 @@ # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction) # s: non-NULL string (e.g., 1st arg to open) # S: optionally-NULL string (e.g., 1st arg to acct) +# U: unsigned long # v: vararg scalar (e.g., optional 3rd arg to open) # V: byte-per-page vector (3rd arg to mincore) # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4) @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do ?:?????????) nargs=9;; esac + # Derive the unsigned long arguments from the argument signature + ulong_arg_1=0 + ulong_arg_2=0 + case $args in + ?:U*) + ulong_arg_1=1 + case $args in + ?:UU*) ulong_arg_2=2;; + ?:U?U*) ulong_arg_2=3;; + ?:U??U*) ulong_arg_2=4;; + ?:U???U*) ulong_arg_2=5;; + ?:U????U*) ulong_arg_2=6;; + ?:U?????U*) ulong_arg_2=7;; + ?:U??????U*) ulong_arg_2=8;; + ?:U???????U) ulong_arg_2=9;; + esac + ;; + ?:?U*) + ulong_arg_1=2 + case $args in + ?:?UU*) ulong_arg_2=3;; + ?:?U?U*) ulong_arg_2=4;; + ?:?U??U*) ulong_arg_2=5;; + ?:?U???U*) ulong_arg_2=6;; + ?:?U????U*) ulong_arg_2=7;; + ?:?U?????U*) ulong_arg_2=8;; + ?:?U??????U) ulong_arg_2=9;; + esac + ;; + ?:??U*) + ulong_arg_1=3 + case $args in + ?:??UU*) ulong_arg_2=4;; + ?:??U?U*) ulong_arg_2=5;; + ?:??U??U*) ulong_arg_2=6;; + ?:??U???U*) ulong_arg_2=7;; + ?:??U????U*) ulong_arg_2=8;; + ?:??U?????U) ulong_arg_2=9;; + esac + ;; + ?:???U*) + ulong_arg_1=4 + case $args in + ?:???UU*) ulong_arg_2=5;; + ?:???U?U*) ulong_arg_2=6;; + ?:???U??U*) ulong_arg_2=7;; + ?:???U???U*) ulong_arg_2=8;; + ?:???U????U) ulong_arg_2=9;; + esac + ;; + ?:????U*) + ulong_arg_1=5 + case $args in + ?:????UU*) ulong_arg_2=6;; + ?:????U?U*) ulong_arg_2=7;; + ?:????U??U*) ulong_arg_2=8;; + ?:????U???U) ulong_arg_2=9;; + esac + ;; + ?:?????U*) + ulong_arg_1=6 + case $args in + ?:?????UU*) ulong_arg_2=7;; + ?:?????U?U*) ulong_arg_2=8;; + ?:?????U??U) ulong_arg_2=9;; + esac + ;; + ?:??????U*) + ulong_arg_1=7 + case $args in + ?:??????UU*) ulong_arg_2=8;; + ?:??????U?U) ulong_arg_2=9;; + esac + ;; + ?:???????U*) + ulong_arg_1=8 + case $args in + ?:??????UU) ulong_arg_2=9;; + esac + ;; + ?:????????U) + ulong_arg_1=9 + ;; + esac + # Make sure only the first syscall rule is used, if multiple dirs # define the same syscall. echo '' @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do \$(make-target-directory) (echo '#define SYSCALL_NAME $syscall'; \\ echo '#define SYSCALL_NARGS $nargs'; \\ + echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\ + echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\ echo '#define SYSCALL_SYMBOL $strong'; \\ echo '#define SYSCALL_NOERRNO $noerrno'; \\ echo '#define SYSCALL_ERRVAL $errval'; \\ diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S index cf6c7a58fb..0824a3c61e 100644 --- a/sysdeps/unix/syscall-template.S +++ b/sysdeps/unix/syscall-template.S @@ -25,6 +25,10 @@ defining a few macros: SYSCALL_NAME syscall name SYSCALL_NARGS number of arguments this call takes + SYSCALL_ULONG_ARG_1 the first unsigned long argument this + call takes + SYSCALL_ULONG_ARG_2 the second unsigned long argument this + call takes SYSCALL_SYMBOL primary symbol name SYSCALL_NOERRNO 1 to define a no-errno version (see below) SYSCALL_ERRVAL 1 to define an error-value version (see below) @@ -44,9 +48,27 @@ /* This indirection is needed so that SYMBOL gets macro-expanded. */ #define syscall_hidden_def(SYMBOL) hidden_def (SYMBOL) -#define T_PSEUDO(SYMBOL, NAME, N) PSEUDO (SYMBOL, NAME, N) -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N) PSEUDO_NOERRNO (SYMBOL, NAME, N) -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N) PSEUDO_ERRVAL (SYMBOL, NAME, N) +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments. */ +#ifndef PSEUDOS_HAVE_4_ARGS +# undef SYSCALL_ULONG_ARG_1 +# define SYSCALL_ULONG_ARG_1 0 +#endif + +#if SYSCALL_ULONG_ARG_1 +# define T_PSEUDO(SYMBOL, NAME, N, U1, U2) \ + PSEUDO (SYMBOL, NAME, N, U1, U2) +# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N, U1, U2) \ + PSEUDO_NOERRNO (SYMBOL, NAME, N, U1, U2) +# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N, U1, U2) \ + PSEUDO_ERRVAL (SYMBOL, NAME, N, U1, U2) +#else +# define T_PSEUDO(SYMBOL, NAME, N) \ + PSEUDO (SYMBOL, NAME, N) +# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N) \ + PSEUDO_NOERRNO (SYMBOL, NAME, N) +# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N) \ + PSEUDO_ERRVAL (SYMBOL, NAME, N) +#endif #define T_PSEUDO_END(SYMBOL) PSEUDO_END (SYMBOL) #define T_PSEUDO_END_NOERRNO(SYMBOL) PSEUDO_END_NOERRNO (SYMBOL) #define T_PSEUDO_END_ERRVAL(SYMBOL) PSEUDO_END_ERRVAL (SYMBOL) @@ -56,7 +78,12 @@ /* This kind of system call stub never returns an error. We return the return value register to the caller unexamined. */ +# if SYSCALL_ULONG_ARG_1 +T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS, + SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2) +# else T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) +# endif ret_NOERRNO T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL) @@ -66,7 +93,12 @@ T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL) value, or zero for success. We may massage the kernel's return value to meet that ABI, but we never set errno here. */ +# if SYSCALL_ULONG_ARG_1 +T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS, + SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2) +# else T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) +# endif ret_ERRVAL T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL) @@ -75,7 +107,12 @@ T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL) /* This is a "normal" system call stub: if there is an error, it returns -1 and sets errno. */ +# if SYSCALL_ULONG_ARG_1 +T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS, + SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2) +# else T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS) +# endif ret T_PSEUDO_END (SYSCALL_SYMBOL) diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list index 01c4a0e6b1..e63a8b9d23 100644 --- a/sysdeps/unix/syscalls.list +++ b/sysdeps/unix/syscalls.list @@ -37,11 +37,11 @@ kill - kill i:ii __kill kill link - link i:ss __link link listen - listen i:ii __listen listen lseek - lseek i:iii __libc_lseek __lseek lseek -madvise - madvise i:pii __madvise madvise +madvise - madvise i:pUi __madvise madvise mkdir - mkdir i:si __mkdir mkdir mmap - mmap b:aniiii __mmap mmap -mprotect - mprotect i:aii __mprotect mprotect -munmap - munmap i:ai __munmap munmap +mprotect - mprotect i:aUi __mprotect mprotect +munmap - munmap i:aU __munmap munmap open - open Ci:siv __libc_open __open open profil - profil i:piii __profil profil ptrace - ptrace i:iiii ptrace diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index e40f993495..4d4af84f9f 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -32,12 +32,12 @@ ioperm - ioperm i:iii ioperm iopl - iopl i:i iopl klogctl EXTRA syslog i:isi klogctl lchown - lchown i:sii __lchown lchown -mincore - mincore i:anV mincore -mlock - mlock i:bn mlock +mincore - mincore i:aUV mincore +mlock - mlock i:bU mlock mlockall - mlockall i:i mlockall -mount EXTRA mount i:sssip __mount mount -mremap EXTRA mremap b:ainip __mremap mremap -munlock - munlock i:ai munlock +mount EXTRA mount i:sssUp __mount mount +mremap EXTRA mremap b:aUUip __mremap mremap +munlock - munlock i:aU munlock munlockall - munlockall i: munlockall nfsservctl EXTRA nfsservctl i:ipp __compat_nfsservctl nfsservctl@GLIBC_2.0:GLIBC_2.28 pipe - pipe i:f __pipe pipe @@ -46,7 +46,7 @@ pivot_root EXTRA pivot_root i:ss pivot_root prctl EXTRA prctl i:iiiii __prctl prctl query_module EXTRA query_module i:sipip __compat_query_module query_module@GLIBC_2.0:GLIBC_2.23 quotactl EXTRA quotactl i:isip quotactl -remap_file_pages - remap_file_pages i:piiii __remap_file_pages remap_file_pages +remap_file_pages - remap_file_pages i:pUiUi __remap_file_pages remap_file_pages sched_getp - sched_getparam i:ip __sched_getparam sched_getparam sched_gets - sched_getscheduler i:i __sched_getscheduler sched_getscheduler sched_primax - sched_get_priority_max i:i __sched_get_priority_max sched_get_priority_max @@ -54,7 +54,7 @@ sched_primin - sched_get_priority_min i:i __sched_get_priority_min sched_get_pri sched_setp - sched_setparam i:ip __sched_setparam sched_setparam sched_sets - sched_setscheduler i:iip __sched_setscheduler sched_setscheduler sched_yield - sched_yield i: __sched_yield sched_yield -sendfile - sendfile i:iipi sendfile +sendfile - sendfile i:iipU sendfile sendfile64 - sendfile64 i:iipi sendfile64 setfsgid EXTRA setfsgid i:i setfsgid setfsuid EXTRA setfsuid i:i setfsuid diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index 24d8b8ec20..06e39212ee 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -57,13 +57,30 @@ # define SYSCALL_ERROR_LABEL syscall_error # endif +/* PSEUDO macros have 4 arguments. */ +# define PSEUDOS_HAVE_4_ARGS 1 + +# ifndef SYSCALL_ULONG_ARG_1 +# define SYSCALL_ULONG_ARG_1 0 +# define SYSCALL_ULONG_ARG_2 0 +# endif + # undef PSEUDO -# define PSEUDO(name, syscall_name, args) \ - .text; \ - ENTRY (name) \ - DO_CALL (syscall_name, args); \ - cmpq $-4095, %rax; \ +# if SYSCALL_ULONG_ARG_1 +# define PSEUDO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2); \ + cmpq $-4095, %rax; \ jae SYSCALL_ERROR_LABEL +# else +# define PSEUDO(name, syscall_name, args) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, 0, 0); \ + cmpq $-4095, %rax; \ + jae SYSCALL_ERROR_LABEL +# endif # undef PSEUDO_END # define PSEUDO_END(name) \ @@ -71,10 +88,17 @@ END (name) # undef PSEUDO_NOERRNO -# define PSEUDO_NOERRNO(name, syscall_name, args) \ - .text; \ - ENTRY (name) \ - DO_CALL (syscall_name, args) +# if SYSCALL_ULONG_ARG_1 +# define PSEUDO_NOERRNO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2) +# else +# define PSEUDO_NOERRNO(name, syscall_name, args) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, 0, 0) +# endif # undef PSEUDO_END_NOERRNO # define PSEUDO_END_NOERRNO(name) \ @@ -83,11 +107,19 @@ # define ret_NOERRNO ret # undef PSEUDO_ERRVAL -# define PSEUDO_ERRVAL(name, syscall_name, args) \ - .text; \ - ENTRY (name) \ - DO_CALL (syscall_name, args); \ +# if SYSCALL_ULONG_ARG_1 +# define PSEUDO_ERRVAL(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2); \ + negq %rax +# else +# define PSEUDO_ERRVAL(name, syscall_name, args) \ + .text; \ + ENTRY (name) \ + DO_CALL (syscall_name, args, 0, 0); \ negq %rax +# endif # undef PSEUDO_END_ERRVAL # define PSEUDO_END_ERRVAL(name) \ @@ -159,8 +191,10 @@ Syscalls of more than 6 arguments are not supported. */ # undef DO_CALL -# define DO_CALL(syscall_name, args) \ +# define DO_CALL(syscall_name, args, ulong_arg_1, ulong_arg_2) \ DOARGS_##args \ + ZERO_EXTEND_##ulong_arg_1 \ + ZERO_EXTEND_##ulong_arg_2 \ movl $SYS_ify (syscall_name), %eax; \ syscall; @@ -172,6 +206,14 @@ # define DOARGS_5 DOARGS_4 # define DOARGS_6 DOARGS_5 +# define ZERO_EXTEND_0 /* nothing */ +# define ZERO_EXTEND_1 /* nothing */ +# define ZERO_EXTEND_2 /* nothing */ +# define ZERO_EXTEND_3 /* nothing */ +# define ZERO_EXTEND_4 /* nothing */ +# define ZERO_EXTEND_5 /* nothing */ +# define ZERO_EXTEND_6 /* nothing */ + #else /* !__ASSEMBLER__ */ /* Registers clobbered by syscall. */ diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h index 5bf9eed80b..2b5cf0c0ea 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h @@ -26,4 +26,24 @@ #undef LO_HI_LONG #define LO_HI_LONG(val) (val) +#ifdef __ASSEMBLER__ +# undef ZERO_EXTEND_1 +# define ZERO_EXTEND_1 movl %edi, %edi; +# undef ZERO_EXTEND_2 +# define ZERO_EXTEND_2 movl %esi, %esi; +# undef ZERO_EXTEND_3 +# define ZERO_EXTEND_3 movl %edx, %edx; +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4 +# undef DOARGS_4 +# define DOARGS_4 movl %ecx, %r10d; +# else +# undef ZERO_EXTEND_4 +# define ZERO_EXTEND_4 movl %r10d, %r10d; +# endif +# undef ZERO_EXTEND_5 +# define ZERO_EXTEND_5 movl %r8d, %r8d; +# undef ZERO_EXTEND_6 +# define ZERO_EXTEND_6 movl %r9d, %r9d; +#endif /* __ASSEMBLER__ */ + #endif /* linux/x86_64/x32/sysdep.h */ -- 2.25.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu 2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu @ 2020-04-13 14:25 ` H.J. Lu 2020-04-13 14:43 ` Florian Weimer 2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu 2 siblings, 1 reply; 13+ messages in thread From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw) To: libc-alpha X32 has 32-bit long and pointer with 64-bit off_t. Since x32 psABI requires that pointers passed in registers must be zero-extended to 64bit, x32 can share many syscall interfaces with LP64. When a LP64 syscall with long and unsigned long arguments is used for x32, these arguments must be properly extended to 64-bit. Otherwise if the upper 32 bits of the register have undefined value, such a syscall will be rejected by kernel. For x32 INLINE_SYSCALLs, cast 1. pointer to unsigned long (32 bit). 2. 32-bit unsigned integer to unsigned long long (64 bit). 3. 32-bit integer to long long (64 bit). For void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset); we now generate 0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d 7: 75 1f jne 28 <__mmap64+0x28> 9: 48 63 d2 movslq %edx,%rdx c: 89 f6 mov %esi,%esi e: 4d 63 c0 movslq %r8d,%r8 11: 4c 63 d1 movslq %ecx,%r10 14: b8 09 00 00 40 mov $0x40000009,%eax 19: 0f 05 syscall That is 1. addr is unchanged. 2. length is zero-extend to 64 bits. 3. prot is sign-extend to 64 bits. 4. flags is sign-extend to 64 bits. 5. fd is sign-extend to 64 bits. 6. offset is unchanged. For int arguments, since kernel uses only the lower 32 bits and ignores the upper 32 bits in 64-bit registers, these work correctly. Tested on x86-64 and x32. There are no code changes on x86-64. --- sysdeps/unix/sysv/linux/x86_64/sysdep.h | 10 ++++++---- sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index 06e39212ee..93dad97c8f 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -219,12 +219,14 @@ /* Registers clobbered by syscall. */ # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" -/* Create a variable 'name' based on type 'X' to avoid explicit types. - This is mainly used set use 64-bits arguments in x32. */ -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name +/* This also works when X is an array. */ +#define TYPEFY1(X) __typeof__ ((X) - (X)) /* Explicit cast the argument to avoid integer from pointer warning on x32. */ -#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X)) +#define ARGIFY(X) ((TYPEFY1 (X)) (X)) +/* Create a variable 'name' based on type 'X' to avoid explicit types. + This is mainly used set use 64-bits arguments in x32. */ +#define TYPEFY(X, name) __typeof__ (ARGIFY (X) - ARGIFY (X)) name #undef INTERNAL_SYSCALL #define INTERNAL_SYSCALL(name, nr, args...) \ diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h index 2b5cf0c0ea..f3bc7e89df 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h @@ -44,6 +44,24 @@ # define ZERO_EXTEND_5 movl %r8d, %r8d; # undef ZERO_EXTEND_6 # define ZERO_EXTEND_6 movl %r9d, %r9d; +#else /*! __ASSEMBLER__ */ +# undef ARGIFY +/* For x32, cast + 1. pointer to unsigned long (32 bit). + 2. 32-bit unsigned integer to unsigned long long (64 bit). + 3. 32-bit integer to long long (64 bit). + */ +# define ARGIFY(X) \ + ({ \ + _Pragma ("GCC diagnostic push"); \ + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ + (__builtin_classify_type (X) == 5 \ + ? (unsigned long) (X) \ + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ + ? (unsigned long long) (X) \ + : (long long) (X))); \ + _Pragma ("GCC diagnostic pop"); \ + }) #endif /* __ASSEMBLER__ */ #endif /* linux/x86_64/x32/sysdep.h */ -- 2.25.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu @ 2020-04-13 14:43 ` Florian Weimer 2020-04-13 15:03 ` H.J. Lu 0 siblings, 1 reply; 13+ messages in thread From: Florian Weimer @ 2020-04-13 14:43 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > index 06e39212ee..93dad97c8f 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > @@ -219,12 +219,14 @@ > /* Registers clobbered by syscall. */ > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" > > -/* Create a variable 'name' based on type 'X' to avoid explicit types. > - This is mainly used set use 64-bits arguments in x32. */ > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name > +/* This also works when X is an array. */ > +#define TYPEFY1(X) __typeof__ ((X) - (X)) I think the comment is now misleading. For an array X, (X) - (X) is of type ptrdiff_t, which is signed, so it triggers sign extension, not zero extension, in the x32 case. I think this is the reason why my proposed construct went astray in your experiments. Is it really necessary to support arrays for system call arguments? > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > index 2b5cf0c0ea..f3bc7e89df 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > @@ -44,6 +44,24 @@ > # define ZERO_EXTEND_5 movl %r8d, %r8d; > # undef ZERO_EXTEND_6 > # define ZERO_EXTEND_6 movl %r9d, %r9d; > +#else /*! __ASSEMBLER__ */ > +# undef ARGIFY > +/* For x32, cast > + 1. pointer to unsigned long (32 bit). > + 2. 32-bit unsigned integer to unsigned long long (64 bit). > + 3. 32-bit integer to long long (64 bit). > + */ > +# define ARGIFY(X) \ > + ({ \ > + _Pragma ("GCC diagnostic push"); \ > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > + (__builtin_classify_type (X) == 5 \ > + ? (unsigned long) (X) \ > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ > + ? (unsigned long long) (X) \ > + : (long long) (X))); \ > + _Pragma ("GCC diagnostic pop"); \ > + }) > #endif /* __ASSEMBLER__ */ This should use long int instead long everywhere. __builtin_classify_type is undocumented. I'm not sure if its behavior and the constant 5 is actually stable across GCC versions, but the powerpc uses it as well, for a slightly difference purpose (to recognize array arguments, it seems). ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 14:43 ` Florian Weimer @ 2020-04-13 15:03 ` H.J. Lu 2020-04-13 15:17 ` Florian Weimer 0 siblings, 1 reply; 13+ messages in thread From: H.J. Lu @ 2020-04-13 15:03 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu via Libc-alpha: > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > index 06e39212ee..93dad97c8f 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > @@ -219,12 +219,14 @@ > > /* Registers clobbered by syscall. */ > > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" > > > > -/* Create a variable 'name' based on type 'X' to avoid explicit types. > > - This is mainly used set use 64-bits arguments in x32. */ > > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name > > +/* This also works when X is an array. */ > > +#define TYPEFY1(X) __typeof__ ((X) - (X)) > > I think the comment is now misleading. For an array X, (X) - (X) is > of type ptrdiff_t, which is signed, so it triggers sign extension, not I will update the comments. > zero extension, in the x32 case. I think this is the reason why my > proposed construct went astray in your experiments. > > Is it really necessary to support arrays for system call arguments? int fexecve (int fd, char *const argv[], char *const envp[]) { if (fd < 0 || argv == NULL || envp == NULL) { __set_errno (EINVAL); return -1; } #ifdef __NR_execveat /* Avoid implicit array coercion in syscall macros. */ INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH); "" is an array. > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > index 2b5cf0c0ea..f3bc7e89df 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > @@ -44,6 +44,24 @@ > > # define ZERO_EXTEND_5 movl %r8d, %r8d; > > # undef ZERO_EXTEND_6 > > # define ZERO_EXTEND_6 movl %r9d, %r9d; > > +#else /*! __ASSEMBLER__ */ > > +# undef ARGIFY > > +/* For x32, cast > > + 1. pointer to unsigned long (32 bit). > > + 2. 32-bit unsigned integer to unsigned long long (64 bit). > > + 3. 32-bit integer to long long (64 bit). > > + */ > > +# define ARGIFY(X) \ > > + ({ \ > > + _Pragma ("GCC diagnostic push"); \ > > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > > + (__builtin_classify_type (X) == 5 \ > > + ? (unsigned long) (X) \ > > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ > > + ? (unsigned long long) (X) \ > > + : (long long) (X))); \ > > + _Pragma ("GCC diagnostic pop"); \ > > + }) > > #endif /* __ASSEMBLER__ */ > > This should use long int instead long everywhere. I will update. > __builtin_classify_type is undocumented. I'm not sure if its behavior > and the constant 5 is actually stable across GCC versions, but the > powerpc uses it as well, for a slightly difference purpose (to > recognize array arguments, it seems). include/libc-pointer-arith.h has /* 1 if 'type' is a pointer type, 0 otherwise. */ # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5) If __builtin_classify_type ever changes, glibc won't build correctly. -- H.J. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 15:03 ` H.J. Lu @ 2020-04-13 15:17 ` Florian Weimer 2020-04-13 16:33 ` V2 " H.J. Lu 0 siblings, 1 reply; 13+ messages in thread From: Florian Weimer @ 2020-04-13 15:17 UTC (permalink / raw) To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha * H. J. Lu: > On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote: >> >> * H. J. Lu via Libc-alpha: >> >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h >> > index 06e39212ee..93dad97c8f 100644 >> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h >> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h >> > @@ -219,12 +219,14 @@ >> > /* Registers clobbered by syscall. */ >> > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" >> > >> > -/* Create a variable 'name' based on type 'X' to avoid explicit types. >> > - This is mainly used set use 64-bits arguments in x32. */ >> > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name >> > +/* This also works when X is an array. */ >> > +#define TYPEFY1(X) __typeof__ ((X) - (X)) >> >> I think the comment is now misleading. For an array X, (X) - (X) is >> of type ptrdiff_t, which is signed, so it triggers sign extension, not > > I will update the comments. Thanks. >> zero extension, in the x32 case. I think this is the reason why my >> proposed construct went astray in your experiments. >> >> Is it really necessary to support arrays for system call arguments? > > int > fexecve (int fd, char *const argv[], char *const envp[]) > { > if (fd < 0 || argv == NULL || envp == NULL) > { > __set_errno (EINVAL); > return -1; > } > > #ifdef __NR_execveat > /* Avoid implicit array coercion in syscall macros. */ > INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH); > > "" is an array. If that were the only reason for enforcing pointer decay, I'd say we'd change the fexecve implementation. But see below. >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > index 2b5cf0c0ea..f3bc7e89df 100644 >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > @@ -44,6 +44,24 @@ >> > # define ZERO_EXTEND_5 movl %r8d, %r8d; >> > # undef ZERO_EXTEND_6 >> > # define ZERO_EXTEND_6 movl %r9d, %r9d; >> > +#else /*! __ASSEMBLER__ */ >> > +# undef ARGIFY >> > +/* For x32, cast >> > + 1. pointer to unsigned long (32 bit). >> > + 2. 32-bit unsigned integer to unsigned long long (64 bit). >> > + 3. 32-bit integer to long long (64 bit). >> > + */ >> > +# define ARGIFY(X) \ >> > + ({ \ >> > + _Pragma ("GCC diagnostic push"); \ >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ >> > + (__builtin_classify_type (X) == 5 \ >> > + ? (unsigned long) (X) \ >> > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ >> > + ? (unsigned long long) (X) \ >> > + : (long long) (X))); \ >> > + _Pragma ("GCC diagnostic pop"); \ >> > + }) >> > #endif /* __ASSEMBLER__ */ >> >> This should use long int instead long everywhere. > > I will update. > >> __builtin_classify_type is undocumented. I'm not sure if its behavior >> and the constant 5 is actually stable across GCC versions, but the >> powerpc uses it as well, for a slightly difference purpose (to >> recognize array arguments, it seems). > > include/libc-pointer-arith.h has > > /* 1 if 'type' is a pointer type, 0 otherwise. */ > # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5) > > If __builtin_classify_type ever changes, glibc won't build correctly. I see. In this case, can we solely rely on __builtin_classify_type? I dont't think the inner ternary operator is necessary. /* Enforce zero-extension for pointers and array system call arguments. For integer types, extend to long long int (the full register) using a regular cast, resulting in zero or sign extension based on the signed-ness of the original type. */ #define ARGIFY(X) \ ({ \ _Pragma ("GCC diagnostic push"); \ _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ (__builtin_classify_type (X) == 5 \ ? (long long int) (uintptr_t) (X) \ : (long long int) (X))); \ _Pragma ("GCC diagnostic pop"); \ }) (Untested, sorry.) ^ permalink raw reply [flat|nested] 13+ messages in thread
* V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 15:17 ` Florian Weimer @ 2020-04-13 16:33 ` H.J. Lu 2020-04-13 17:00 ` Florian Weimer 2020-04-13 18:34 ` Adhemerval Zanella 0 siblings, 2 replies; 13+ messages in thread From: H.J. Lu @ 2020-04-13 16:33 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 5001 bytes --] On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu: > > > On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote: > >> > >> * H. J. Lu via Libc-alpha: > >> > >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >> > index 06e39212ee..93dad97c8f 100644 > >> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >> > @@ -219,12 +219,14 @@ > >> > /* Registers clobbered by syscall. */ > >> > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" > >> > > >> > -/* Create a variable 'name' based on type 'X' to avoid explicit types. > >> > - This is mainly used set use 64-bits arguments in x32. */ > >> > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name > >> > +/* This also works when X is an array. */ > >> > +#define TYPEFY1(X) __typeof__ ((X) - (X)) > >> > >> I think the comment is now misleading. For an array X, (X) - (X) is > >> of type ptrdiff_t, which is signed, so it triggers sign extension, not > > > > I will update the comments. > > Thanks. > > >> zero extension, in the x32 case. I think this is the reason why my > >> proposed construct went astray in your experiments. > >> > >> Is it really necessary to support arrays for system call arguments? > > > > int > > fexecve (int fd, char *const argv[], char *const envp[]) > > { > > if (fd < 0 || argv == NULL || envp == NULL) > > { > > __set_errno (EINVAL); > > return -1; > > } > > > > #ifdef __NR_execveat > > /* Avoid implicit array coercion in syscall macros. */ > > INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH); > > > > "" is an array. > > If that were the only reason for enforcing pointer decay, I'd say we'd > change the fexecve implementation. But see below. > > >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > index 2b5cf0c0ea..f3bc7e89df 100644 > >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > @@ -44,6 +44,24 @@ > >> > # define ZERO_EXTEND_5 movl %r8d, %r8d; > >> > # undef ZERO_EXTEND_6 > >> > # define ZERO_EXTEND_6 movl %r9d, %r9d; > >> > +#else /*! __ASSEMBLER__ */ > >> > +# undef ARGIFY > >> > +/* For x32, cast > >> > + 1. pointer to unsigned long (32 bit). > >> > + 2. 32-bit unsigned integer to unsigned long long (64 bit). > >> > + 3. 32-bit integer to long long (64 bit). > >> > + */ > >> > +# define ARGIFY(X) \ > >> > + ({ \ > >> > + _Pragma ("GCC diagnostic push"); \ > >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > >> > + (__builtin_classify_type (X) == 5 \ > >> > + ? (unsigned long) (X) \ > >> > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ > >> > + ? (unsigned long long) (X) \ > >> > + : (long long) (X))); \ > >> > + _Pragma ("GCC diagnostic pop"); \ > >> > + }) > >> > #endif /* __ASSEMBLER__ */ > >> > >> This should use long int instead long everywhere. > > > > I will update. > > > >> __builtin_classify_type is undocumented. I'm not sure if its behavior > >> and the constant 5 is actually stable across GCC versions, but the > >> powerpc uses it as well, for a slightly difference purpose (to > >> recognize array arguments, it seems). > > > > include/libc-pointer-arith.h has > > > > /* 1 if 'type' is a pointer type, 0 otherwise. */ > > # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5) > > > > If __builtin_classify_type ever changes, glibc won't build correctly. > > I see. In this case, can we solely rely on __builtin_classify_type? > I dont't think the inner ternary operator is necessary. > > /* Enforce zero-extension for pointers and array system call > arguments. For integer types, extend to long long int (the full > register) using a regular cast, resulting in zero or sign extension > based on the signed-ness of the original type. */ > #define ARGIFY(X) \ > ({ \ > _Pragma ("GCC diagnostic push"); \ > _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > (__builtin_classify_type (X) == 5 \ > ? (long long int) (uintptr_t) (X) \ > : (long long int) (X))); \ > _Pragma ("GCC diagnostic pop"); \ > }) > > (Untested, sorry.) Here is the updated patch. -- H.J. [-- Attachment #2: 0002-x32-Properly-pass-long-to-syscall-BZ-25810.patch --] [-- Type: text/x-patch, Size: 4189 bytes --] From fd210e7ada784986f5f5bff3e4892478fc998fea Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Mon, 13 Apr 2020 07:02:46 -0700 Subject: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] X32 has 32-bit long and pointer with 64-bit off_t. Since x32 psABI requires that pointers passed in registers must be zero-extended to 64bit, x32 can share many syscall interfaces with LP64. When a LP64 syscall with long and unsigned long arguments is used for x32, these arguments must be properly extended to 64-bit. Otherwise if the upper 32 bits of the register have undefined value, such a syscall will be rejected by kernel. Enforce zero-extension for pointers and array system call arguments. For integer types, extend to int64_t (the full register) using a regular cast, resulting in zero or sign extension based on the signedness of the original type. For void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset); we now generate 0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d 7: 75 1f jne 28 <__mmap64+0x28> 9: 48 63 d2 movslq %edx,%rdx c: 89 f6 mov %esi,%esi e: 4d 63 c0 movslq %r8d,%r8 11: 4c 63 d1 movslq %ecx,%r10 14: b8 09 00 00 40 mov $0x40000009,%eax 19: 0f 05 syscall That is 1. addr is unchanged. 2. length is zero-extend to 64 bits. 3. prot is sign-extend to 64 bits. 4. flags is sign-extend to 64 bits. 5. fd is sign-extend to 64 bits. 6. offset is unchanged. For int arguments, since kernel uses only the lower 32 bits and ignores the upper 32 bits in 64-bit registers, these work correctly. Tested on x86-64 and x32. There are no code changes on x86-64. --- sysdeps/unix/sysv/linux/x86_64/sysdep.h | 15 +++++++++------ sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index 06e39212ee..3596273c94 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -219,12 +219,15 @@ /* Registers clobbered by syscall. */ # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" -/* Create a variable 'name' based on type 'X' to avoid explicit types. - This is mainly used set use 64-bits arguments in x32. */ -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name -/* Explicit cast the argument to avoid integer from pointer warning on - x32. */ -#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X)) +/* NB: This also works when X is an array. For an array X, type of + (X) - (X) is ptrdiff_t, which is signed, since size of ptrdiff_t + == size of pointer, cast is a NOP. */ +#define TYPEFY1(X) __typeof__ ((X) - (X)) +/* Explicit cast the argument. */ +#define ARGIFY(X) ((TYPEFY1 (X)) (X)) +/* Create a variable 'name' based on type of variable 'X' to avoid + explicit types. */ +#define TYPEFY(X, name) __typeof__ (ARGIFY (X)) name #undef INTERNAL_SYSCALL #define INTERNAL_SYSCALL(name, nr, args...) \ diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h index 2b5cf0c0ea..67335c2a7f 100644 --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h @@ -44,6 +44,20 @@ # define ZERO_EXTEND_5 movl %r8d, %r8d; # undef ZERO_EXTEND_6 # define ZERO_EXTEND_6 movl %r9d, %r9d; +#else /*! __ASSEMBLER__ */ +# undef ARGIFY +/* Enforce zero-extension for pointers and array system call arguments. + For integer types, extend to int64_t (the full register) using a + regular cast, resulting in zero or sign extension based on the + signedness of the original type. */ +# define ARGIFY(X) \ + ({ \ + _Pragma ("GCC diagnostic push"); \ + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ + (__builtin_classify_type (X) == 5 \ + ? (uintptr_t) (X) : (int64_t) (X)); \ + _Pragma ("GCC diagnostic pop"); \ + }) #endif /* __ASSEMBLER__ */ #endif /* linux/x86_64/x32/sysdep.h */ -- 2.25.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 16:33 ` V2 " H.J. Lu @ 2020-04-13 17:00 ` Florian Weimer 2020-04-13 18:34 ` Adhemerval Zanella 1 sibling, 0 replies; 13+ messages in thread From: Florian Weimer @ 2020-04-13 17:00 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Mon, 13 Apr 2020 07:02:46 -0700 > Subject: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] > > X32 has 32-bit long and pointer with 64-bit off_t. Since x32 psABI > requires that pointers passed in registers must be zero-extended to > 64bit, x32 can share many syscall interfaces with LP64. When a LP64 > syscall with long and unsigned long arguments is used for x32, these > arguments must be properly extended to 64-bit. Otherwise if the upper > 32 bits of the register have undefined value, such a syscall will be > rejected by kernel. This version looks okay to me, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 16:33 ` V2 " H.J. Lu 2020-04-13 17:00 ` Florian Weimer @ 2020-04-13 18:34 ` Adhemerval Zanella 2020-04-13 20:43 ` H.J. Lu 1 sibling, 1 reply; 13+ messages in thread From: Adhemerval Zanella @ 2020-04-13 18:34 UTC (permalink / raw) To: libc-alpha On 13/04/2020 13:33, H.J. Lu via Libc-alpha wrote: > On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote: >> >> * H. J. Lu: >> >>> On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote: >>>> >>>> * H. J. Lu via Libc-alpha: >>>> >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h >>>>> index 06e39212ee..93dad97c8f 100644 >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h >>>>> @@ -219,12 +219,14 @@ >>>>> /* Registers clobbered by syscall. */ >>>>> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" >>>>> >>>>> -/* Create a variable 'name' based on type 'X' to avoid explicit types. >>>>> - This is mainly used set use 64-bits arguments in x32. */ >>>>> -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name >>>>> +/* This also works when X is an array. */ >>>>> +#define TYPEFY1(X) __typeof__ ((X) - (X)) >>>> >>>> I think the comment is now misleading. For an array X, (X) - (X) is >>>> of type ptrdiff_t, which is signed, so it triggers sign extension, not >>> >>> I will update the comments. >> >> Thanks. >> >>>> zero extension, in the x32 case. I think this is the reason why my >>>> proposed construct went astray in your experiments. >>>> >>>> Is it really necessary to support arrays for system call arguments? >>> >>> int >>> fexecve (int fd, char *const argv[], char *const envp[]) >>> { >>> if (fd < 0 || argv == NULL || envp == NULL) >>> { >>> __set_errno (EINVAL); >>> return -1; >>> } >>> >>> #ifdef __NR_execveat >>> /* Avoid implicit array coercion in syscall macros. */ >>> INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH); >>> >>> "" is an array. >> >> If that were the only reason for enforcing pointer decay, I'd say we'd >> change the fexecve implementation. But see below. >> >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >>>>> index 2b5cf0c0ea..f3bc7e89df 100644 >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >>>>> @@ -44,6 +44,24 @@ >>>>> # define ZERO_EXTEND_5 movl %r8d, %r8d; >>>>> # undef ZERO_EXTEND_6 >>>>> # define ZERO_EXTEND_6 movl %r9d, %r9d; >>>>> +#else /*! __ASSEMBLER__ */ >>>>> +# undef ARGIFY >>>>> +/* For x32, cast >>>>> + 1. pointer to unsigned long (32 bit). >>>>> + 2. 32-bit unsigned integer to unsigned long long (64 bit). >>>>> + 3. 32-bit integer to long long (64 bit). >>>>> + */ >>>>> +# define ARGIFY(X) \ >>>>> + ({ \ >>>>> + _Pragma ("GCC diagnostic push"); \ >>>>> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ >>>>> + (__builtin_classify_type (X) == 5 \ >>>>> + ? (unsigned long) (X) \ >>>>> + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ >>>>> + ? (unsigned long long) (X) \ >>>>> + : (long long) (X))); \ >>>>> + _Pragma ("GCC diagnostic pop"); \ >>>>> + }) >>>>> #endif /* __ASSEMBLER__ */ >>>> >>>> This should use long int instead long everywhere. >>> >>> I will update. >>> >>>> __builtin_classify_type is undocumented. I'm not sure if its behavior >>>> and the constant 5 is actually stable across GCC versions, but the >>>> powerpc uses it as well, for a slightly difference purpose (to >>>> recognize array arguments, it seems). >>> >>> include/libc-pointer-arith.h has >>> >>> /* 1 if 'type' is a pointer type, 0 otherwise. */ >>> # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5) >>> >>> If __builtin_classify_type ever changes, glibc won't build correctly. >> >> I see. In this case, can we solely rely on __builtin_classify_type? >> I dont't think the inner ternary operator is necessary. >> >> /* Enforce zero-extension for pointers and array system call >> arguments. For integer types, extend to long long int (the full >> register) using a regular cast, resulting in zero or sign extension >> based on the signed-ness of the original type. */ >> #define ARGIFY(X) \ >> ({ \ >> _Pragma ("GCC diagnostic push"); \ >> _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ >> (__builtin_classify_type (X) == 5 \ >> ? (long long int) (uintptr_t) (X) \ >> : (long long int) (X))); \ >> _Pragma ("GCC diagnostic pop"); \ >> }) >> >> (Untested, sorry.) > > Here is the updated patch. > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > index 2b5cf0c0ea..67335c2a7f 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > @@ -44,6 +44,20 @@ > # define ZERO_EXTEND_5 movl %r8d, %r8d; > # undef ZERO_EXTEND_6 > # define ZERO_EXTEND_6 movl %r9d, %r9d; > +#else /*! __ASSEMBLER__ */ > +# undef ARGIFY > +/* Enforce zero-extension for pointers and array system call arguments. > + For integer types, extend to int64_t (the full register) using a > + regular cast, resulting in zero or sign extension based on the > + signedness of the original type. */ > +# define ARGIFY(X) \ > + ({ \ > + _Pragma ("GCC diagnostic push"); \ > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > + (__builtin_classify_type (X) == 5 \ > + ? (uintptr_t) (X) : (int64_t) (X)); \ > + _Pragma ("GCC diagnostic pop"); \ > + }) > #endif /* __ASSEMBLER__ */ > > #endif /* linux/x86_64/x32/sysdep.h */ I face a similar issue when fixing BZ#12683 for x32, and one possible solution I found was: #define __SSC(__x) \ ({ \ __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \ ? (unsigned long int) (uintptr_t)(__x) \ : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \ __arg; \ }) Which was for argument passing in the internal cancellable bridge. Later you suggested a solution I have also devised sometime ago [1]. Couldn't we use any of them instead of explicitly disable/enable compile warnings? [1] https://sourceware.org/legacy-ml/libc-alpha/2017-12/msg00315.html ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 18:34 ` Adhemerval Zanella @ 2020-04-13 20:43 ` H.J. Lu 2020-04-13 21:13 ` Florian Weimer 0 siblings, 1 reply; 13+ messages in thread From: H.J. Lu @ 2020-04-13 20:43 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GNU C Library On Mon, Apr 13, 2020 at 11:35 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 13/04/2020 13:33, H.J. Lu via Libc-alpha wrote: > > On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote: > >> > >> * H. J. Lu: > >> > >>> On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote: > >>>> > >>>> * H. J. Lu via Libc-alpha: > >>>> > >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >>>>> index 06e39212ee..93dad97c8f 100644 > >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > >>>>> @@ -219,12 +219,14 @@ > >>>>> /* Registers clobbered by syscall. */ > >>>>> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx" > >>>>> > >>>>> -/* Create a variable 'name' based on type 'X' to avoid explicit types. > >>>>> - This is mainly used set use 64-bits arguments in x32. */ > >>>>> -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name > >>>>> +/* This also works when X is an array. */ > >>>>> +#define TYPEFY1(X) __typeof__ ((X) - (X)) > >>>> > >>>> I think the comment is now misleading. For an array X, (X) - (X) is > >>>> of type ptrdiff_t, which is signed, so it triggers sign extension, not > >>> > >>> I will update the comments. > >> > >> Thanks. > >> > >>>> zero extension, in the x32 case. I think this is the reason why my > >>>> proposed construct went astray in your experiments. > >>>> > >>>> Is it really necessary to support arrays for system call arguments? > >>> > >>> int > >>> fexecve (int fd, char *const argv[], char *const envp[]) > >>> { > >>> if (fd < 0 || argv == NULL || envp == NULL) > >>> { > >>> __set_errno (EINVAL); > >>> return -1; > >>> } > >>> > >>> #ifdef __NR_execveat > >>> /* Avoid implicit array coercion in syscall macros. */ > >>> INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH); > >>> > >>> "" is an array. > >> > >> If that were the only reason for enforcing pointer decay, I'd say we'd > >> change the fexecve implementation. But see below. > >> > >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >>>>> index 2b5cf0c0ea..f3bc7e89df 100644 > >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >>>>> @@ -44,6 +44,24 @@ > >>>>> # define ZERO_EXTEND_5 movl %r8d, %r8d; > >>>>> # undef ZERO_EXTEND_6 > >>>>> # define ZERO_EXTEND_6 movl %r9d, %r9d; > >>>>> +#else /*! __ASSEMBLER__ */ > >>>>> +# undef ARGIFY > >>>>> +/* For x32, cast > >>>>> + 1. pointer to unsigned long (32 bit). > >>>>> + 2. 32-bit unsigned integer to unsigned long long (64 bit). > >>>>> + 3. 32-bit integer to long long (64 bit). > >>>>> + */ > >>>>> +# define ARGIFY(X) \ > >>>>> + ({ \ > >>>>> + _Pragma ("GCC diagnostic push"); \ > >>>>> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > >>>>> + (__builtin_classify_type (X) == 5 \ > >>>>> + ? (unsigned long) (X) \ > >>>>> + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \ > >>>>> + ? (unsigned long long) (X) \ > >>>>> + : (long long) (X))); \ > >>>>> + _Pragma ("GCC diagnostic pop"); \ > >>>>> + }) > >>>>> #endif /* __ASSEMBLER__ */ > >>>> > >>>> This should use long int instead long everywhere. > >>> > >>> I will update. > >>> > >>>> __builtin_classify_type is undocumented. I'm not sure if its behavior > >>>> and the constant 5 is actually stable across GCC versions, but the > >>>> powerpc uses it as well, for a slightly difference purpose (to > >>>> recognize array arguments, it seems). > >>> > >>> include/libc-pointer-arith.h has > >>> > >>> /* 1 if 'type' is a pointer type, 0 otherwise. */ > >>> # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5) > >>> > >>> If __builtin_classify_type ever changes, glibc won't build correctly. > >> > >> I see. In this case, can we solely rely on __builtin_classify_type? > >> I dont't think the inner ternary operator is necessary. > >> > >> /* Enforce zero-extension for pointers and array system call > >> arguments. For integer types, extend to long long int (the full > >> register) using a regular cast, resulting in zero or sign extension > >> based on the signed-ness of the original type. */ > >> #define ARGIFY(X) \ > >> ({ \ > >> _Pragma ("GCC diagnostic push"); \ > >> _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > >> (__builtin_classify_type (X) == 5 \ > >> ? (long long int) (uintptr_t) (X) \ > >> : (long long int) (X))); \ > >> _Pragma ("GCC diagnostic pop"); \ > >> }) > >> > >> (Untested, sorry.) > > > > Here is the updated patch. > > > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > index 2b5cf0c0ea..67335c2a7f 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > > @@ -44,6 +44,20 @@ > > # define ZERO_EXTEND_5 movl %r8d, %r8d; > > # undef ZERO_EXTEND_6 > > # define ZERO_EXTEND_6 movl %r9d, %r9d; > > +#else /*! __ASSEMBLER__ */ > > +# undef ARGIFY > > +/* Enforce zero-extension for pointers and array system call arguments. > > + For integer types, extend to int64_t (the full register) using a > > + regular cast, resulting in zero or sign extension based on the > > + signedness of the original type. */ > > +# define ARGIFY(X) \ > > + ({ \ > > + _Pragma ("GCC diagnostic push"); \ > > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > > + (__builtin_classify_type (X) == 5 \ > > + ? (uintptr_t) (X) : (int64_t) (X)); \ > > + _Pragma ("GCC diagnostic pop"); \ > > + }) > > #endif /* __ASSEMBLER__ */ > > > > #endif /* linux/x86_64/x32/sysdep.h */ > > I face a similar issue when fixing BZ#12683 for x32, and one possible > solution I found was: > > #define __SSC(__x) \ > ({ \ > __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \ > ? (unsigned long int) (uintptr_t)(__x) \ > : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \ > __arg; \ > }) This is similar to what I have checked in. > Which was for argument passing in the internal cancellable bridge. > Later you suggested a solution I have also devised sometime ago [1]. > > Couldn't we use any of them instead of explicitly disable/enable > compile warnings? > > [1] https://sourceware.org/legacy-ml/libc-alpha/2017-12/msg00315.html This doesn't work on array, like "". -- H.J. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 20:43 ` H.J. Lu @ 2020-04-13 21:13 ` Florian Weimer 2020-04-13 21:19 ` H.J. Lu 0 siblings, 1 reply; 13+ messages in thread From: Florian Weimer @ 2020-04-13 21:13 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > index 2b5cf0c0ea..67335c2a7f 100644 >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h >> > @@ -44,6 +44,20 @@ >> > # define ZERO_EXTEND_5 movl %r8d, %r8d; >> > # undef ZERO_EXTEND_6 >> > # define ZERO_EXTEND_6 movl %r9d, %r9d; >> > +#else /*! __ASSEMBLER__ */ >> > +# undef ARGIFY >> > +/* Enforce zero-extension for pointers and array system call arguments. >> > + For integer types, extend to int64_t (the full register) using a >> > + regular cast, resulting in zero or sign extension based on the >> > + signedness of the original type. */ >> > +# define ARGIFY(X) \ >> > + ({ \ >> > + _Pragma ("GCC diagnostic push"); \ >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ >> > + (__builtin_classify_type (X) == 5 \ >> > + ? (uintptr_t) (X) : (int64_t) (X)); \ >> > + _Pragma ("GCC diagnostic pop"); \ >> > + }) >> > #endif /* __ASSEMBLER__ */ >> > >> > #endif /* linux/x86_64/x32/sysdep.h */ >> >> I face a similar issue when fixing BZ#12683 for x32, and one possible >> solution I found was: >> >> #define __SSC(__x) \ >> ({ \ >> __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \ >> ? (unsigned long int) (uintptr_t)(__x) \ >> : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \ >> __arg; \ >> }) > > This is similar to what I have checked in. I think Adhemerval specifically meant the additional cast using __typeof__ to suppress the warning (which I did not think of). The _Pragma construct as written is a Clang porting hazard because it causes the statement expression to return void (which is a Clang/GCC discrepancy). Rather than introducing a temporary, using the __typeof__ hack is probably the better way of fixing it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810] 2020-04-13 21:13 ` Florian Weimer @ 2020-04-13 21:19 ` H.J. Lu 0 siblings, 0 replies; 13+ messages in thread From: H.J. Lu @ 2020-04-13 21:19 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Adhemerval Zanella On Mon, Apr 13, 2020 at 2:13 PM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu via Libc-alpha: > > >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > index 2b5cf0c0ea..67335c2a7f 100644 > >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h > >> > @@ -44,6 +44,20 @@ > >> > # define ZERO_EXTEND_5 movl %r8d, %r8d; > >> > # undef ZERO_EXTEND_6 > >> > # define ZERO_EXTEND_6 movl %r9d, %r9d; > >> > +#else /*! __ASSEMBLER__ */ > >> > +# undef ARGIFY > >> > +/* Enforce zero-extension for pointers and array system call arguments. > >> > + For integer types, extend to int64_t (the full register) using a > >> > + regular cast, resulting in zero or sign extension based on the > >> > + signedness of the original type. */ > >> > +# define ARGIFY(X) \ > >> > + ({ \ > >> > + _Pragma ("GCC diagnostic push"); \ > >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \ > >> > + (__builtin_classify_type (X) == 5 \ > >> > + ? (uintptr_t) (X) : (int64_t) (X)); \ > >> > + _Pragma ("GCC diagnostic pop"); \ > >> > + }) > >> > #endif /* __ASSEMBLER__ */ > >> > > >> > #endif /* linux/x86_64/x32/sysdep.h */ > >> > >> I face a similar issue when fixing BZ#12683 for x32, and one possible > >> solution I found was: > >> > >> #define __SSC(__x) \ > >> ({ \ > >> __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \ > >> ? (unsigned long int) (uintptr_t)(__x) \ > >> : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \ > >> __arg; \ > >> }) > > > > This is similar to what I have checked in. > > I think Adhemerval specifically meant the additional cast using > __typeof__ to suppress the warning (which I did not think of). His patch: https://sourceware.org/pipermail/libc-alpha/2020-April/112520.html also needs to suppress -Wpointer-to-int-cast > The _Pragma construct as written is a Clang porting hazard because it > causes the statement expression to return void (which is a Clang/GCC > discrepancy). Rather than introducing a temporary, using the > __typeof__ hack is probably the better way of fixing it. I don't think it works on array. -- H.J. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] Add a syscall test for [BZ #25810] 2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu 2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu 2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu @ 2020-04-13 14:25 ` H.J. Lu 2 siblings, 0 replies; 13+ messages in thread From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw) To: libc-alpha Add a test to pass 64-bit long arguments to syscall with undefined upper 32 bits on ILP32 systems. Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py. --- misc/Makefile | 2 +- misc/tst-syscalls.c | 146 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 misc/tst-syscalls.c diff --git a/misc/Makefile b/misc/Makefile index b8fed5783d..67c5237f97 100644 --- a/misc/Makefile +++ b/misc/Makefile @@ -87,7 +87,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \ tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \ tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \ tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \ - tst-mntent-autofs + tst-mntent-autofs tst-syscalls # Tests which need libdl. ifeq (yes,$(build-shared)) diff --git a/misc/tst-syscalls.c b/misc/tst-syscalls.c new file mode 100644 index 0000000000..d07f03633b --- /dev/null +++ b/misc/tst-syscalls.c @@ -0,0 +1,146 @@ +/* Test for syscall interfaces. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <fcntl.h> +#include <sys/mman.h> +#include <support/check.h> +#include <support/xunistd.h> + +struct Array +{ + size_t length; + void *ptr; +}; + +static int error_count; + +__attribute__ ((noclone, noinline)) +struct Array +allocate (size_t bytes) +{ + if (!bytes) + return __extension__ (struct Array) {0, 0}; + + void *p = mmap (0x0, bytes, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON, -1, 0); + if (p == MAP_FAILED) + return __extension__ (struct Array) {0, 0}; + + return __extension__ (struct Array) {bytes, p}; +} + +__attribute__ ((noclone, noinline)) +void +deallocate (struct Array b) +{ + if (b.length && munmap (b.ptr, b.length)) + { + printf ("munmap error: %m\n"); + error_count++; + } +} + +__attribute__ ((noclone, noinline)) +void * +do_mmap (void *addr, size_t length) +{ + return mmap (addr, length, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON, -1, 0); +} + +__attribute__ ((noclone, noinline)) +void * +reallocate (struct Array b) +{ + if (b.length) + return do_mmap (b.ptr, b.length); + return NULL; +} + +__attribute__ ((noclone, noinline)) +void +protect (struct Array b) +{ + if (b.length) + { + if (mprotect (b.ptr, b.length, + PROT_READ | PROT_WRITE | PROT_EXEC)) + { + printf ("mprotect error: %m\n"); + error_count++; + } + } +} + +__attribute__ ((noclone, noinline)) +ssize_t +do_read (int fd, void *ptr, struct Array b) +{ + if (b.length) + return read (fd, ptr, b.length); + return 0; +} + +__attribute__ ((noclone, noinline)) +ssize_t +do_write (int fd, void *ptr, struct Array b) +{ + if (b.length) + return write (fd, ptr, b.length); + return 0; +} + +static int +do_test (void) +{ + struct Array array; + + array = allocate (1); + protect (array); + deallocate (array); + void *p = reallocate (array); + if (p == MAP_FAILED) + { + printf ("mmap error: %m\n"); + error_count++; + } + array.ptr = p; + protect (array); + deallocate (array); + + int fd = xopen ("/dev/null", O_RDWR, 0); + char buf[2]; + array.ptr = buf; + if (do_read (fd, array.ptr, array) == -1) + { + printf ("read error: %m\n"); + error_count++; + } + if (do_write (fd, array.ptr, array) == -1) + { + printf ("write error: %m\n"); + error_count++; + } + xclose (fd); + + return error_count ? EXIT_FAILURE : EXIT_SUCCESS; +} + +#include <support/test-driver.c> -- 2.25.2 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-04-13 21:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu 2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu 2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu 2020-04-13 14:43 ` Florian Weimer 2020-04-13 15:03 ` H.J. Lu 2020-04-13 15:17 ` Florian Weimer 2020-04-13 16:33 ` V2 " H.J. Lu 2020-04-13 17:00 ` Florian Weimer 2020-04-13 18:34 ` Adhemerval Zanella 2020-04-13 20:43 ` H.J. Lu 2020-04-13 21:13 ` Florian Weimer 2020-04-13 21:19 ` H.J. Lu 2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).