* [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall @ 2023-04-24 15:03 Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-04-24 15:03 UTC (permalink / raw) To: libc-alpha; +Cc: Joe Simmons-Talbott To make binary call tree analysis easier place the syscall number into the register just before the syscall is made. Only do this if the syscall number is a constant integer. Compiler optimizations can place quite a few instructions between the setting of the syscall number and the syscall instruction. During call tree analysis the number of instructions between the two can lead to more difficulty for both tools and humans in properly identifying the syscall number. Having the syscall number set in the prior instruction to the syscall instruction makes this task easier and less error prone. Being able to reliably identify syscalls made by a given API will make it easier to understand and verify the safety and security of glibc. Tested on x86_64 and aarch64. Chnages to v5: * aarch64: Remove '\n\t' from asm. * x86_64: Remove unused macros. Changes to v4: * aarch64: Move generated comment. Remove unused macros from earlier version. * Added patch - nptl: Convert indirect syscall numbers to compile time numeric values. Changes to v3: * x86_64: Combine two inline asms into one. - Suggested by "H.J. Lu" <hjl.tools@gmail.com> Changes to v2: * Add a more detailed commit message. Changes to v1: * aarch64: Combine two inline asms into one. Avoid loading name twice. Joe Simmons-Talbott (3): x86_64: Set the syscall register right before doing the syscall. aarch64: Set the syscall register right before doing the syscall. nptl: Use direct syscall numbers in setxid nptl/nptl_setxid.c | 136 ++++++++++++++++++++++- sysdeps/unix/sysv/linux/aarch64/sysdep.h | 29 +++-- sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++-- 3 files changed, 164 insertions(+), 25 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-04-24 15:03 [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott @ 2023-04-24 15:03 ` Joe Simmons-Talbott 2023-05-15 14:15 ` Joe Simmons-Talbott 2023-05-25 18:07 ` Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 2/3] aarch64: " Joe Simmons-Talbott ` (2 subsequent siblings) 3 siblings, 2 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-04-24 15:03 UTC (permalink / raw) To: libc-alpha; +Cc: Joe Simmons-Talbott To make identifying syscalls easier during call tree analysis load the syscall number just before performing the syscall. Compiler optimizations can place quite a few instructions between the setting of the syscall number and the syscall instruction. During call tree analysis the number of instructions between the two can lead to more difficulty for both tools and humans in properly identifying the syscall number. Having the syscall number set in the prior instruction to the syscall instruction makes this task easier and less error prone. Being able to reliably identify syscalls made by a given API will make it easier to understand and verify the safety and security of glibc. --- sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h index cfb51be8c5..0db8660531 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h @@ -257,9 +257,9 @@ TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1) \ + : "g" (number), "r" (_a1) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ }) @@ -273,9 +273,9 @@ register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1), "r" (_a2) \ + : "g" (number), "r" (_a1), "r" (_a2) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ }) @@ -291,9 +291,9 @@ register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ }) @@ -311,9 +311,9 @@ register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ }) @@ -333,9 +333,9 @@ register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ "r" (_a5) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ @@ -358,9 +358,9 @@ register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ asm volatile ( \ - "syscall\n\t" \ + "movl %1, %k0\n\tsyscall\n\t" \ : "=a" (resultvar) \ - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ "r" (_a5), "r" (_a6) \ : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ (long int) resultvar; \ -- 2.39.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott @ 2023-05-15 14:15 ` Joe Simmons-Talbott 2023-05-15 16:20 ` H.J. Lu 2023-05-25 18:07 ` Joe Simmons-Talbott 1 sibling, 1 reply; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-05-15 14:15 UTC (permalink / raw) To: libc-alpha, H.J. Lu Hi H.J., Is there anything else you are looking for on x86_64 WRT this patch? Thanks, Joe On Mon, Apr 24, 2023 at 11:03:51AM -0400, Joe Simmons-Talbott wrote: > To make identifying syscalls easier during call tree analysis load the > syscall number just before performing the syscall. > > Compiler optimizations can place quite a few instructions between the > setting of the syscall number and the syscall instruction. During call > tree analysis the number of instructions between the two can lead to > more difficulty for both tools and humans in properly identifying the > syscall number. Having the syscall number set in the prior instruction > to the syscall instruction makes this task easier and less error prone. > Being able to reliably identify syscalls made by a given API will make > it easier to understand and verify the safety and security of glibc. > --- > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > index cfb51be8c5..0db8660531 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > @@ -257,9 +257,9 @@ > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1) \ > + : "g" (number), "r" (_a1) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -273,9 +273,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2) \ > + : "g" (number), "r" (_a1), "r" (_a2) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -291,9 +291,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -311,9 +311,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -333,9 +333,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > "r" (_a5) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > @@ -358,9 +358,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > "r" (_a5), "r" (_a6) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-15 14:15 ` Joe Simmons-Talbott @ 2023-05-15 16:20 ` H.J. Lu 0 siblings, 0 replies; 22+ messages in thread From: H.J. Lu @ 2023-05-15 16:20 UTC (permalink / raw) To: Joe Simmons-Talbott; +Cc: libc-alpha On Mon, May 15, 2023 at 7:15 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote: > > Hi H.J., > > Is there anything else you are looking for on x86_64 WRT this patch? > > Thanks, > Joe > On Mon, Apr 24, 2023 at 11:03:51AM -0400, Joe Simmons-Talbott wrote: > > To make identifying syscalls easier during call tree analysis load the > > syscall number just before performing the syscall. > > > > Compiler optimizations can place quite a few instructions between the > > setting of the syscall number and the syscall instruction. During call > > tree analysis the number of instructions between the two can lead to > > more difficulty for both tools and humans in properly identifying the > > syscall number. Having the syscall number set in the prior instruction > > to the syscall instruction makes this task easier and less error prone. > > Being able to reliably identify syscalls made by a given API will make > > it easier to understand and verify the safety and security of glibc. > > --- > > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++++++++++++------------ > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > index cfb51be8c5..0db8660531 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > @@ -257,9 +257,9 @@ > > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1) \ > > + : "g" (number), "r" (_a1) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -273,9 +273,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2) \ > > + : "g" (number), "r" (_a1), "r" (_a2) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -291,9 +291,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -311,9 +311,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -333,9 +333,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > "r" (_a5) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > @@ -358,9 +358,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > "r" (_a5), "r" (_a6) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > -- > > 2.39.2 > > > I have no more comments. -- H.J. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott 2023-05-15 14:15 ` Joe Simmons-Talbott @ 2023-05-25 18:07 ` Joe Simmons-Talbott 2023-05-25 18:40 ` Noah Goldstein 1 sibling, 1 reply; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-05-25 18:07 UTC (permalink / raw) To: libc-alpha ping. Thanks, Joe On Mon, Apr 24, 2023 at 11:03:51AM -0400, Joe Simmons-Talbott wrote: > To make identifying syscalls easier during call tree analysis load the > syscall number just before performing the syscall. > > Compiler optimizations can place quite a few instructions between the > setting of the syscall number and the syscall instruction. During call > tree analysis the number of instructions between the two can lead to > more difficulty for both tools and humans in properly identifying the > syscall number. Having the syscall number set in the prior instruction > to the syscall instruction makes this task easier and less error prone. > Being able to reliably identify syscalls made by a given API will make > it easier to understand and verify the safety and security of glibc. > --- > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > index cfb51be8c5..0db8660531 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > @@ -257,9 +257,9 @@ > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1) \ > + : "g" (number), "r" (_a1) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -273,9 +273,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2) \ > + : "g" (number), "r" (_a1), "r" (_a2) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -291,9 +291,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -311,9 +311,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > }) > @@ -333,9 +333,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > "r" (_a5) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > @@ -358,9 +358,9 @@ > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > asm volatile ( \ > - "syscall\n\t" \ > + "movl %1, %k0\n\tsyscall\n\t" \ > : "=a" (resultvar) \ > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > "r" (_a5), "r" (_a6) \ > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > (long int) resultvar; \ > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-25 18:07 ` Joe Simmons-Talbott @ 2023-05-25 18:40 ` Noah Goldstein 2023-05-26 7:04 ` Florian Weimer 0 siblings, 1 reply; 22+ messages in thread From: Noah Goldstein @ 2023-05-25 18:40 UTC (permalink / raw) To: Joe Simmons-Talbott; +Cc: libc-alpha On Thu, May 25, 2023 at 1:07 PM Joe Simmons-Talbott via Libc-alpha <libc-alpha@sourceware.org> wrote: > > ping. > > Thanks, > Joe > On Mon, Apr 24, 2023 at 11:03:51AM -0400, Joe Simmons-Talbott wrote: > > To make identifying syscalls easier during call tree analysis load the > > syscall number just before performing the syscall. > > > > Compiler optimizations can place quite a few instructions between the > > setting of the syscall number and the syscall instruction. During call > > tree analysis the number of instructions between the two can lead to > > more difficulty for both tools and humans in properly identifying the > > syscall number. Having the syscall number set in the prior instruction > > to the syscall instruction makes this task easier and less error prone. > > Being able to reliably identify syscalls made by a given API will make > > it easier to understand and verify the safety and security of glibc. > > --- > > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++++++++++++------------ > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > index cfb51be8c5..0db8660531 100644 > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h > > @@ -257,9 +257,9 @@ > > TYPEFY (arg1, __arg1) = ARGIFY (arg1); \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1) \ > > + : "g" (number), "r" (_a1) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -273,9 +273,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2) \ > > + : "g" (number), "r" (_a1), "r" (_a2) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -291,9 +291,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -311,9 +311,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > }) > > @@ -333,9 +333,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > "r" (_a5) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > @@ -358,9 +358,9 @@ > > register TYPEFY (arg2, _a2) asm ("rsi") = __arg2; \ > > register TYPEFY (arg1, _a1) asm ("rdi") = __arg1; \ > > asm volatile ( \ > > - "syscall\n\t" \ > > + "movl %1, %k0\n\tsyscall\n\t" \ > > : "=a" (resultvar) \ > > - : "0" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > + : "g" (number), "r" (_a1), "r" (_a2), "r" (_a3), "r" (_a4), \ > > "r" (_a5), "r" (_a6) \ > > : "memory", REGISTERS_CLOBBERED_BY_SYSCALL); \ > > (long int) resultvar; \ > > -- > > 2.39.2 > > > I'm minorly opposed to this patch. Even if GLIBC guarantees all syscalls will set the number the instruction before, that's no guarantee for the entire program. Furthermore in the event of: `movl $VAL, %eax; syscall` It's still not safe to *always* assume that `VAL` correspond to the syscall number as a jump (direct or indirect) could still go between the instructions (i.e there is no guarantee in the assembly that the `mov` dominates the `syscall). So at the end of the day, we are bloating the library without, AFAICT, providing any real guarantee. Maybe I'm missing something? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-25 18:40 ` Noah Goldstein @ 2023-05-26 7:04 ` Florian Weimer 2023-05-26 12:59 ` Joe Simmons-Talbott 0 siblings, 1 reply; 22+ messages in thread From: Florian Weimer @ 2023-05-26 7:04 UTC (permalink / raw) To: Noah Goldstein via Libc-alpha; +Cc: Joe Simmons-Talbott, Noah Goldstein * Noah Goldstein via Libc-alpha: > I'm minorly opposed to this patch. Even if GLIBC guarantees all > syscalls will set the number the instruction before, that's no guarantee > for the entire program. Furthermore in the event of: > `movl $VAL, %eax; syscall` > It's still not safe to *always* assume that `VAL` correspond to the > syscall number as a jump (direct or indirect) could still go between > the instructions (i.e there is no guarantee in the assembly that the > `mov` dominates the `syscall). > So at the end of the day, we are bloating the library without, AFAICT, > providing any real guarantee. Maybe I'm missing something? Joe, is there a size change to libc.so.6 as the result of this change? Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-26 7:04 ` Florian Weimer @ 2023-05-26 12:59 ` Joe Simmons-Talbott 2023-05-26 21:18 ` Noah Goldstein 0 siblings, 1 reply; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-05-26 12:59 UTC (permalink / raw) To: Florian Weimer; +Cc: Noah Goldstein via Libc-alpha, Noah Goldstein On Fri, May 26, 2023 at 09:04:06AM +0200, Florian Weimer wrote: > * Noah Goldstein via Libc-alpha: > > > I'm minorly opposed to this patch. Even if GLIBC guarantees all > > syscalls will set the number the instruction before, that's no guarantee > > for the entire program. Furthermore in the event of: > > `movl $VAL, %eax; syscall` > > It's still not safe to *always* assume that `VAL` correspond to the > > syscall number as a jump (direct or indirect) could still go between > > the instructions (i.e there is no guarantee in the assembly that the > > `mov` dominates the `syscall). > > So at the end of the day, we are bloating the library without, AFAICT, > > providing any real guarantee. Maybe I'm missing something? > > Joe, is there a size change to libc.so.6 as the result of this change? No, the size is the same with and with out this patchset on x86_64. Thanks, Joe ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-26 12:59 ` Joe Simmons-Talbott @ 2023-05-26 21:18 ` Noah Goldstein 2023-05-30 10:13 ` Florian Weimer 0 siblings, 1 reply; 22+ messages in thread From: Noah Goldstein @ 2023-05-26 21:18 UTC (permalink / raw) To: Joe Simmons-Talbott; +Cc: Florian Weimer, Noah Goldstein via Libc-alpha On Fri, May 26, 2023 at 5:59 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote: > > On Fri, May 26, 2023 at 09:04:06AM +0200, Florian Weimer wrote: > > * Noah Goldstein via Libc-alpha: > > > > > I'm minorly opposed to this patch. Even if GLIBC guarantees all > > > syscalls will set the number the instruction before, that's no guarantee > > > for the entire program. Furthermore in the event of: > > > `movl $VAL, %eax; syscall` > > > It's still not safe to *always* assume that `VAL` correspond to the > > > syscall number as a jump (direct or indirect) could still go between > > > the instructions (i.e there is no guarantee in the assembly that the > > > `mov` dominates the `syscall). > > > So at the end of the day, we are bloating the library without, AFAICT, > > > providing any real guarantee. Maybe I'm missing something? > > > > Joe, is there a size change to libc.so.6 as the result of this change? > > No, the size is the same with and with out this patchset on x86_64. > There aren't many syscalls so it's only a minor cost (hence the only minor opposition), but I don't see the value this provides given that it still won't be safe to assume the syscall number is always set the instruction beforehand for any robust purpose. So it still feels like why take any cost at all? > Thanks, > Joe > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-26 21:18 ` Noah Goldstein @ 2023-05-30 10:13 ` Florian Weimer 2023-05-31 18:23 ` Noah Goldstein 0 siblings, 1 reply; 22+ messages in thread From: Florian Weimer @ 2023-05-30 10:13 UTC (permalink / raw) To: Noah Goldstein; +Cc: Joe Simmons-Talbott, Noah Goldstein via Libc-alpha * Noah Goldstein: > On Fri, May 26, 2023 at 5:59 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote: >> >> On Fri, May 26, 2023 at 09:04:06AM +0200, Florian Weimer wrote: >> > * Noah Goldstein via Libc-alpha: >> > >> > > I'm minorly opposed to this patch. Even if GLIBC guarantees all >> > > syscalls will set the number the instruction before, that's no guarantee >> > > for the entire program. Furthermore in the event of: >> > > `movl $VAL, %eax; syscall` >> > > It's still not safe to *always* assume that `VAL` correspond to the >> > > syscall number as a jump (direct or indirect) could still go between >> > > the instructions (i.e there is no guarantee in the assembly that the >> > > `mov` dominates the `syscall). >> > > So at the end of the day, we are bloating the library without, AFAICT, >> > > providing any real guarantee. Maybe I'm missing something? >> > >> > Joe, is there a size change to libc.so.6 as the result of this change? >> >> No, the size is the same with and with out this patchset on x86_64. >> > There aren't many syscalls so it's only a minor cost (hence the only > minor opposition), but I don't see the value this provides given that it > still won't be safe to assume the syscall number is always set the > instruction beforehand for any robust purpose. So it still feels like > why take any cost at all? I think there is any run-time cost at all, only the increased source complexity. It's much easier to change glibc than to add full register tracking to a the static analysis tool that discovers system calls in the disassembly. Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-30 10:13 ` Florian Weimer @ 2023-05-31 18:23 ` Noah Goldstein 2023-06-28 19:17 ` Joe Simmons-Talbott 0 siblings, 1 reply; 22+ messages in thread From: Noah Goldstein @ 2023-05-31 18:23 UTC (permalink / raw) To: Florian Weimer; +Cc: Joe Simmons-Talbott, Noah Goldstein via Libc-alpha On Tue, May 30, 2023 at 5:13 AM Florian Weimer <fweimer@redhat.com> wrote: > > * Noah Goldstein: > > > On Fri, May 26, 2023 at 5:59 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote: > >> > >> On Fri, May 26, 2023 at 09:04:06AM +0200, Florian Weimer wrote: > >> > * Noah Goldstein via Libc-alpha: > >> > > >> > > I'm minorly opposed to this patch. Even if GLIBC guarantees all > >> > > syscalls will set the number the instruction before, that's no guarantee > >> > > for the entire program. Furthermore in the event of: > >> > > `movl $VAL, %eax; syscall` > >> > > It's still not safe to *always* assume that `VAL` correspond to the > >> > > syscall number as a jump (direct or indirect) could still go between > >> > > the instructions (i.e there is no guarantee in the assembly that the > >> > > `mov` dominates the `syscall). > >> > > So at the end of the day, we are bloating the library without, AFAICT, > >> > > providing any real guarantee. Maybe I'm missing something? > >> > > >> > Joe, is there a size change to libc.so.6 as the result of this change? > >> > >> No, the size is the same with and with out this patchset on x86_64. > >> > > There aren't many syscalls so it's only a minor cost (hence the only > > minor opposition), but I don't see the value this provides given that it > > still won't be safe to assume the syscall number is always set the > > instruction beforehand for any robust purpose. So it still feels like > > why take any cost at all? > > I think there is any run-time cost at all, only the increased source > complexity. > > It's much easier to change glibc than to add full register tracking to a > the static analysis tool that discovers system calls in the disassembly. > Is the aim only to verify libc.so or to verify arbitrary binaries? If the former then sure I guess we know we only use the syscall wrapper so this may help (more on that), but if it's arbitrary binaries there is no guarantee they are using the GLIBC syscall wrapper for their syscalls. If it really is just GLIBC then this change seems unnecessary. Even if there can be separation between setting rax and the syscall, the way we have the wrappers setup we know there will always be a dominating write to rax with the syscall number so would rather see the case where that isn't trivial to find as a motivator first. Or we could just do source code level analysis as we will always have the syscall number in macro invocation. > Thanks, > Florian > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall. 2023-05-31 18:23 ` Noah Goldstein @ 2023-06-28 19:17 ` Joe Simmons-Talbott 0 siblings, 0 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-06-28 19:17 UTC (permalink / raw) To: Noah Goldstein; +Cc: Florian Weimer, Noah Goldstein via Libc-alpha On Wed, May 31, 2023 at 01:23:44PM -0500, Noah Goldstein wrote: > On Tue, May 30, 2023 at 5:13 AM Florian Weimer <fweimer@redhat.com> wrote: > > > > * Noah Goldstein: > > > > > On Fri, May 26, 2023 at 5:59 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote: > > >> > > >> On Fri, May 26, 2023 at 09:04:06AM +0200, Florian Weimer wrote: > > >> > * Noah Goldstein via Libc-alpha: > > >> > > > >> > > I'm minorly opposed to this patch. Even if GLIBC guarantees all > > >> > > syscalls will set the number the instruction before, that's no guarantee > > >> > > for the entire program. Furthermore in the event of: > > >> > > `movl $VAL, %eax; syscall` > > >> > > It's still not safe to *always* assume that `VAL` correspond to the > > >> > > syscall number as a jump (direct or indirect) could still go between > > >> > > the instructions (i.e there is no guarantee in the assembly that the > > >> > > `mov` dominates the `syscall). > > >> > > So at the end of the day, we are bloating the library without, AFAICT, > > >> > > providing any real guarantee. Maybe I'm missing something? > > >> > > > >> > Joe, is there a size change to libc.so.6 as the result of this change? > > >> > > >> No, the size is the same with and with out this patchset on x86_64. > > >> > > > There aren't many syscalls so it's only a minor cost (hence the only > > > minor opposition), but I don't see the value this provides given that it > > > still won't be safe to assume the syscall number is always set the > > > instruction beforehand for any robust purpose. So it still feels like > > > why take any cost at all? > > > > I think there is any run-time cost at all, only the increased source > > complexity. > > > > It's much easier to change glibc than to add full register tracking to a > > the static analysis tool that discovers system calls in the disassembly. > > > > Is the aim only to verify libc.so or to verify arbitrary binaries? If the > former then sure I guess we know we only use the syscall wrapper > so this may help (more on that), but if it's arbitrary binaries there > is no guarantee they are using the GLIBC syscall wrapper for their > syscalls. > > If it really is just GLIBC then this change seems unnecessary. Even > if there can be separation between setting rax and the syscall, the > way we have the wrappers setup we know there will always be a > dominating write to rax with the syscall number so would rather see > the case where that isn't trivial to find as a motivator first. Or we could > just do source code level analysis as we will always have the > syscall number in macro invocation. > Right now we are only concerned with libc.so. Here's an example case from my installed libc.so. 000000000003ec70 <__GI___arc4random_buf.part.0>: 3ec70: 55 push %rbp 3ec71: 41 b8 3e 01 00 00 mov $0x13e,%r8d 3ec77: 48 89 e5 mov %rsp,%rbp 3ec7a: 41 56 push %r14 3ec7c: 41 55 push %r13 3ec7e: 41 54 push %r12 3ec80: 49 89 fc mov %rdi,%r12 3ec83: 53 push %rbx 3ec84: 48 89 f3 mov %rsi,%rbx 3ec87: 48 83 ec 10 sub $0x10,%rsp 3ec8b: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax 3ec92: 00 00 3ec94: 48 89 45 d8 mov %rax,-0x28(%rbp) 3ec98: 31 c0 xor %eax,%eax 3ec9a: 31 d2 xor %edx,%edx 3ec9c: 48 89 de mov %rbx,%rsi 3ec9f: 4c 89 e7 mov %r12,%rdi 3eca2: 44 89 c0 mov %r8d,%eax 3eca5: 0f 05 syscall And with my patchset: 0000000000039480 <__GI___arc4random_buf.part.0>: 39480: 41 55 push %r13 39482: 41 54 push %r12 39484: 55 push %rbp 39485: 48 89 fd mov %rdi,%rbp 39488: 53 push %rbx 39489: 48 89 f3 mov %rsi,%rbx 3948c: 48 83 ec 18 sub $0x18,%rsp 39490: 31 d2 xor %edx,%edx 39492: 48 89 de mov %rbx,%rsi 39495: 48 89 ef mov %rbp,%rdi 39498: b8 3e 01 00 00 mov $0x13e,%eax 3949d: 0f 05 syscall Thanks, Joe ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v6 2/3] aarch64: Set the syscall register right before doing the syscall. 2023-04-24 15:03 [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott @ 2023-04-24 15:03 ` Joe Simmons-Talbott 2023-05-09 7:47 ` Szabolcs Nagy 2023-04-24 15:03 ` [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott 2023-05-08 14:13 ` [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott 3 siblings, 1 reply; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-04-24 15:03 UTC (permalink / raw) To: libc-alpha; +Cc: Joe Simmons-Talbott To make identifying syscalls easier during call tree analysis load the syscall number just before performing the syscall. Compiler optimizations can place quite a few instructions between the setting of the syscall number and the syscall instruction. During call tree analysis the number of instructions between the two can lead to more difficulty for both tools and humans in properly identifying the syscall number. Having the syscall number set in the prior instruction to the syscall instruction makes this task easier and less error prone. Being able to reliably identify syscalls made by a given API will make it easier to understand and verify the safety and security of glibc. --- sysdeps/unix/sysv/linux/aarch64/sysdep.h | 29 ++++++++++++++++-------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h index e94d1703ad..6fe40aaf89 100644 --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h @@ -168,15 +168,26 @@ # define HAVE_CLONE3_WRAPPER 1 # undef INTERNAL_SYSCALL_RAW -# define INTERNAL_SYSCALL_RAW(name, nr, args...) \ - ({ long _sys_result; \ - { \ - LOAD_ARGS_##nr (args) \ - register long _x8 asm ("x8") = (name); \ - asm volatile ("svc 0 // syscall " # name \ - : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ - _sys_result = _x0; \ - } \ +# define INTERNAL_SYSCALL_RAW(name, nr, args...) \ + ({ long _sys_result; \ + { \ + LOAD_ARGS_##nr (args) \ + if (__builtin_constant_p(name)) \ + asm volatile ("mov x8, %1 // syscall " # name "\n" \ + "svc 0" \ + : "=r" (_x0) \ + : "i" (name) ASM_ARGS_##nr \ + : "x8", "memory"); \ + else \ + { \ + register long _x8 asm ("x8") = (name); \ + asm volatile ("svc 0" \ + : "=r" (_x0) \ + : "r"(_x8) ASM_ARGS_##nr \ + : "memory"); \ + } \ + _sys_result = _x0; \ + } \ _sys_result; }) # undef INTERNAL_SYSCALL -- 2.39.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 2/3] aarch64: Set the syscall register right before doing the syscall. 2023-04-24 15:03 ` [PATCH v6 2/3] aarch64: " Joe Simmons-Talbott @ 2023-05-09 7:47 ` Szabolcs Nagy 0 siblings, 0 replies; 22+ messages in thread From: Szabolcs Nagy @ 2023-05-09 7:47 UTC (permalink / raw) To: Joe Simmons-Talbott, libc-alpha The 04/24/2023 11:03, Joe Simmons-Talbott via Libc-alpha wrote: > To make identifying syscalls easier during call tree analysis load the > syscall number just before performing the syscall. > > Compiler optimizations can place quite a few instructions between the > setting of the syscall number and the syscall instruction. During call > tree analysis the number of instructions between the two can lead to > more difficulty for both tools and humans in properly identifying the > syscall number. Having the syscall number set in the prior instruction > to the syscall instruction makes this task easier and less error prone. > Being able to reliably identify syscalls made by a given API will make > it easier to understand and verify the safety and security of glibc. This is OK to commit. Reviewed-by: Szabolcs Nagy <szabolcs.nagy@arm.com> > --- > sysdeps/unix/sysv/linux/aarch64/sysdep.h | 29 ++++++++++++++++-------- > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h > index e94d1703ad..6fe40aaf89 100644 > --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h > +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h > @@ -168,15 +168,26 @@ > # define HAVE_CLONE3_WRAPPER 1 > > # undef INTERNAL_SYSCALL_RAW > -# define INTERNAL_SYSCALL_RAW(name, nr, args...) \ > - ({ long _sys_result; \ > - { \ > - LOAD_ARGS_##nr (args) \ > - register long _x8 asm ("x8") = (name); \ > - asm volatile ("svc 0 // syscall " # name \ > - : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory"); \ > - _sys_result = _x0; \ > - } \ > +# define INTERNAL_SYSCALL_RAW(name, nr, args...) \ > + ({ long _sys_result; \ > + { \ > + LOAD_ARGS_##nr (args) \ > + if (__builtin_constant_p(name)) \ > + asm volatile ("mov x8, %1 // syscall " # name "\n" \ > + "svc 0" \ > + : "=r" (_x0) \ > + : "i" (name) ASM_ARGS_##nr \ > + : "x8", "memory"); \ > + else \ > + { \ > + register long _x8 asm ("x8") = (name); \ > + asm volatile ("svc 0" \ > + : "=r" (_x0) \ > + : "r"(_x8) ASM_ARGS_##nr \ > + : "memory"); \ > + } \ > + _sys_result = _x0; \ > + } \ > _sys_result; }) > > # undef INTERNAL_SYSCALL > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-24 15:03 [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 2/3] aarch64: " Joe Simmons-Talbott @ 2023-04-24 15:03 ` Joe Simmons-Talbott 2023-04-24 15:17 ` Xi Ruoyao 2023-05-25 18:07 ` Joe Simmons-Talbott 2023-05-08 14:13 ` [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott 3 siblings, 2 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-04-24 15:03 UTC (permalink / raw) To: libc-alpha; +Cc: Joe Simmons-Talbott Make all internal glibc syscalls use direct compile time numeric values rather than variables. This will make the syscall number easier to identify during static analysis. --- nptl/nptl_setxid.c | 136 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 4 deletions(-) diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c index 4bfcfe4188..797015def1 100644 --- a/nptl/nptl_setxid.c +++ b/nptl/nptl_setxid.c @@ -66,8 +66,72 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, void *ctx) || si->si_code != SI_TKILL) return; - result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0], - xidcmd->id[1], xidcmd->id[2]); + switch(xidcmd->syscall_no) + { +#ifdef __NR_setuid32 + case __NR_setuid32: + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setuid32 */ + case __NR_setuid: + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#ifdef __NR_setreuid32 + case __NR_setreuid32: + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setreuid32 */ + case __NR_setreuid: + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#ifdef __NR_setresgid32 + case __NR_setresgid32: + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setresgid32 */ + case __NR_setresgid: + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#ifdef __NR_setregid32 + case __NR_setregid32: + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setregid32 */ + case __NR_setregid: + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#ifdef __NR_setgid32 + case __NR_setgid32: + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setgid32 */ + case __NR_setgid: + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#ifdef __NR_setgroups32 + case __NR_setgroups32: + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; +#endif /* __NR_setgroups32 */ + case __NR_setgroups: + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + break; + default: + result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0], + xidcmd->id[1], xidcmd->id[2]); + } int error = 0; if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) error = INTERNAL_SYSCALL_ERRNO (result); @@ -262,8 +326,72 @@ __nptl_setxid (struct xid_command *cmdp) /* This must be last, otherwise the current thread might not have permissions to send SIGSETXID syscall to the other threads. */ - result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, - cmdp->id[0], cmdp->id[1], cmdp->id[2]); + switch(cmdp->syscall_no) + { +#ifdef __NR_setuid32 + case __NR_setuid32: + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setuid32 */ + case __NR_setuid: + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#ifdef __NR_setreuid32 + case __NR_setreuid32: + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setreuid32 */ + case __NR_setreuid: + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#ifdef __NR_setresgid32 + case __NR_setresgid32: + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setresgid32 */ + case __NR_setresgid: + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#ifdef __NR_setregid32 + case __NR_setregid32: + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setregid32 */ + case __NR_setregid: + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#ifdef __NR_setgid32 + case __NR_setgid32: + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setgid32 */ + case __NR_setgid: + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#ifdef __NR_setgroups32 + case __NR_setgroups32: + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; +#endif /* __NR_setgroups32 */ + case __NR_setgroups: + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + break; + default: + result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, cmdp->id[0], + cmdp->id[1], cmdp->id[2]); + } int error = 0; if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) { -- 2.39.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-24 15:03 ` [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott @ 2023-04-24 15:17 ` Xi Ruoyao 2023-04-26 9:46 ` Szabolcs Nagy 2023-04-26 12:39 ` Cristian Rodríguez 2023-05-25 18:07 ` Joe Simmons-Talbott 1 sibling, 2 replies; 22+ messages in thread From: Xi Ruoyao @ 2023-04-24 15:17 UTC (permalink / raw) To: Joe Simmons-Talbott, libc-alpha On Mon, 2023-04-24 at 11:03 -0400, Joe Simmons-Talbott via Libc-alpha wrote: > Make all internal glibc syscalls use direct compile time numeric values > rather than variables. This will make the syscall number easier to > identify during static analysis. This is making the code much more bloated and slower. Do we really want to make everyone's system slower for some debug tools? > --- > nptl/nptl_setxid.c | 136 +++++++++++++++++++++++++++++++++++++++++++- > - > 1 file changed, 132 insertions(+), 4 deletions(-) > > diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c > index 4bfcfe4188..797015def1 100644 > --- a/nptl/nptl_setxid.c > +++ b/nptl/nptl_setxid.c > @@ -66,8 +66,72 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, > void *ctx) > || si->si_code != SI_TKILL) > return; > > - result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd- > >id[0], > - xidcmd->id[1], xidcmd->id[2]); > + switch(xidcmd->syscall_no) > + { > +#ifdef __NR_setuid32 > + case __NR_setuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setuid32 */ > + case __NR_setuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setreuid32 > + case __NR_setreuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setreuid32 */ > + case __NR_setreuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setresgid32 > + case __NR_setresgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setresgid32 */ > + case __NR_setresgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setregid32 > + case __NR_setregid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setregid32 */ > + case __NR_setregid: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setgid32 > + case __NR_setgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setgid32 */ > + case __NR_setgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setgroups32 > + case __NR_setgroups32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setgroups32 */ > + case __NR_setgroups: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > + default: > + result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd- > >id[0], > + xidcmd->id[1], xidcmd->id[2]); > + } > int error = 0; > if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) > error = INTERNAL_SYSCALL_ERRNO (result); > @@ -262,8 +326,72 @@ __nptl_setxid (struct xid_command *cmdp) > > /* This must be last, otherwise the current thread might not have > permissions to send SIGSETXID syscall to the other threads. */ > - result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, > - cmdp->id[0], cmdp->id[1], cmdp- > >id[2]); > + switch(cmdp->syscall_no) > + { > +#ifdef __NR_setuid32 > + case __NR_setuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setuid32 */ > + case __NR_setuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setreuid32 > + case __NR_setreuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setreuid32 */ > + case __NR_setreuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setresgid32 > + case __NR_setresgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, cmdp- > >id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setresgid32 */ > + case __NR_setresgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setregid32 > + case __NR_setregid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setregid32 */ > + case __NR_setregid: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setgid32 > + case __NR_setgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setgid32 */ > + case __NR_setgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setgroups32 > + case __NR_setgroups32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, cmdp- > >id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setgroups32 */ > + case __NR_setgroups: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > + default: > + result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, cmdp- > >id[0], > + cmdp->id[1], cmdp->id[2]); > + } > int error = 0; > if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) > { -- Xi Ruoyao <xry111@xry111.site> School of Aerospace Science and Technology, Xidian University ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-24 15:17 ` Xi Ruoyao @ 2023-04-26 9:46 ` Szabolcs Nagy 2023-04-28 10:52 ` Florian Weimer 2023-04-26 12:39 ` Cristian Rodríguez 1 sibling, 1 reply; 22+ messages in thread From: Szabolcs Nagy @ 2023-04-26 9:46 UTC (permalink / raw) To: Xi Ruoyao, Joe Simmons-Talbott, libc-alpha The 04/24/2023 23:17, Xi Ruoyao via Libc-alpha wrote: > On Mon, 2023-04-24 at 11:03 -0400, Joe Simmons-Talbott via Libc-alpha > wrote: > > Make all internal glibc syscalls use direct compile time numeric values > > rather than variables. This will make the syscall number easier to > > identify during static analysis. > > This is making the code much more bloated and slower. Do we really want > to make everyone's system slower for some debug tools? the switch statement overhead is many orders of magnitude smaller than sending a signal to a thread and executing a syscall there (which is where the switch statement happens). i dont know if the change is justified, but from a target port perspective it's useful if inline syscalls are guaranteed to use a syscall name that can expand to an integer constant or named function. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-26 9:46 ` Szabolcs Nagy @ 2023-04-28 10:52 ` Florian Weimer 0 siblings, 0 replies; 22+ messages in thread From: Florian Weimer @ 2023-04-28 10:52 UTC (permalink / raw) To: Szabolcs Nagy via Libc-alpha Cc: Xi Ruoyao, Joe Simmons-Talbott, Szabolcs Nagy * Szabolcs Nagy via Libc-alpha: > The 04/24/2023 23:17, Xi Ruoyao via Libc-alpha wrote: >> On Mon, 2023-04-24 at 11:03 -0400, Joe Simmons-Talbott via Libc-alpha >> wrote: >> > Make all internal glibc syscalls use direct compile time numeric values >> > rather than variables. This will make the syscall number easier to >> > identify during static analysis. >> >> This is making the code much more bloated and slower. Do we really want >> to make everyone's system slower for some debug tools? > > the switch statement overhead is many orders of magnitude smaller > than sending a signal to a thread and executing a syscall there > (which is where the switch statement happens). > > i dont know if the change is justified, but from a target port > perspective it's useful if inline syscalls are guaranteed to use > a syscall name that can expand to an integer constant or named > function. If the only place we have dynamic system calls in glibc is the syscall function, we can add a check there to block system calls using it unless the symbol has been bound before. Together with BTI/IBT, this would prevent issuing arbitrary system calls using machine code fragments from glibc. Not sure if that is important consideration. If it does not matter, we could change the pthread code to call an internal alias of the syscall function instead. Thanks, Florian ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-24 15:17 ` Xi Ruoyao 2023-04-26 9:46 ` Szabolcs Nagy @ 2023-04-26 12:39 ` Cristian Rodríguez 2023-04-26 13:24 ` Szabolcs Nagy 1 sibling, 1 reply; 22+ messages in thread From: Cristian Rodríguez @ 2023-04-26 12:39 UTC (permalink / raw) To: Xi Ruoyao; +Cc: Joe Simmons-Talbott, libc-alpha [-- Attachment #1: Type: text/plain, Size: 744 bytes --] On Mon, Apr 24, 2023 at 11:17 AM Xi Ruoyao via Libc-alpha < libc-alpha@sourceware.org> wrote: > On Mon, 2023-04-24 at 11:03 -0400, Joe Simmons-Talbott via Libc-alpha > wrote: > > Make all internal glibc syscalls use direct compile time numeric values > > rather than variables. This will make the syscall number easier to > > identify during static analysis. > > This is making the code much more bloated and slower. Do we really want > to make everyone's system slower for some debug tools? > > The switch statement will add little overhead. my concern with this is that it looks ugly and error prone for no other reason than help some limited debug tool.. there must be a nicer, less verbose way to get what you want.. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-26 12:39 ` Cristian Rodríguez @ 2023-04-26 13:24 ` Szabolcs Nagy 0 siblings, 0 replies; 22+ messages in thread From: Szabolcs Nagy @ 2023-04-26 13:24 UTC (permalink / raw) To: Cristian Rodríguez, Xi Ruoyao; +Cc: Joe Simmons-Talbott, libc-alpha The 04/26/2023 08:39, Cristian Rodríguez via Libc-alpha wrote: > On Mon, Apr 24, 2023 at 11:17 AM Xi Ruoyao via Libc-alpha < > libc-alpha@sourceware.org> wrote: > > > On Mon, 2023-04-24 at 11:03 -0400, Joe Simmons-Talbott via Libc-alpha > > wrote: > > > Make all internal glibc syscalls use direct compile time numeric values > > > rather than variables. This will make the syscall number easier to > > > identify during static analysis. > > > > This is making the code much more bloated and slower. Do we really want > > to make everyone's system slower for some debug tools? > > > > > The switch statement will add little overhead. my concern with this is that > it looks ugly and error prone for no other reason than help some > limited debug tool.. there must be a nicer, less verbose way to get what > you want.. it allows glibc to drop the INTERNAL_SYSCALL_NCS code. which can clean up some target code. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid 2023-04-24 15:03 ` [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott 2023-04-24 15:17 ` Xi Ruoyao @ 2023-05-25 18:07 ` Joe Simmons-Talbott 1 sibling, 0 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-05-25 18:07 UTC (permalink / raw) To: libc-alpha ping. Thanks, Joe On Mon, Apr 24, 2023 at 11:03:53AM -0400, Joe Simmons-Talbott wrote: > Make all internal glibc syscalls use direct compile time numeric values > rather than variables. This will make the syscall number easier to > identify during static analysis. > --- > nptl/nptl_setxid.c | 136 +++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 132 insertions(+), 4 deletions(-) > > diff --git a/nptl/nptl_setxid.c b/nptl/nptl_setxid.c > index 4bfcfe4188..797015def1 100644 > --- a/nptl/nptl_setxid.c > +++ b/nptl/nptl_setxid.c > @@ -66,8 +66,72 @@ __nptl_setxid_sighandler (int sig, siginfo_t *si, void *ctx) > || si->si_code != SI_TKILL) > return; > > - result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0], > - xidcmd->id[1], xidcmd->id[2]); > + switch(xidcmd->syscall_no) > + { > +#ifdef __NR_setuid32 > + case __NR_setuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setuid32 */ > + case __NR_setuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setreuid32 > + case __NR_setreuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setreuid32 */ > + case __NR_setreuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setresgid32 > + case __NR_setresgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setresgid32 */ > + case __NR_setresgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setregid32 > + case __NR_setregid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setregid32 */ > + case __NR_setregid: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setgid32 > + case __NR_setgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setgid32 */ > + case __NR_setgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#ifdef __NR_setgroups32 > + case __NR_setgroups32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > +#endif /* __NR_setgroups32 */ > + case __NR_setgroups: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + break; > + default: > + result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0], > + xidcmd->id[1], xidcmd->id[2]); > + } > int error = 0; > if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) > error = INTERNAL_SYSCALL_ERRNO (result); > @@ -262,8 +326,72 @@ __nptl_setxid (struct xid_command *cmdp) > > /* This must be last, otherwise the current thread might not have > permissions to send SIGSETXID syscall to the other threads. */ > - result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, > - cmdp->id[0], cmdp->id[1], cmdp->id[2]); > + switch(cmdp->syscall_no) > + { > +#ifdef __NR_setuid32 > + case __NR_setuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setuid32 */ > + case __NR_setuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setuid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setreuid32 > + case __NR_setreuid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setreuid32 */ > + case __NR_setreuid: > + result = INTERNAL_SYSCALL_NCS (__NR_setreuid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setresgid32 > + case __NR_setresgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setresgid32 */ > + case __NR_setresgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setresgid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setregid32 > + case __NR_setregid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setregid32 */ > + case __NR_setregid: > + result = INTERNAL_SYSCALL_NCS (__NR_setregid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setgid32 > + case __NR_setgid32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setgid32 */ > + case __NR_setgid: > + result = INTERNAL_SYSCALL_NCS (__NR_setgid, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#ifdef __NR_setgroups32 > + case __NR_setgroups32: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups32, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > +#endif /* __NR_setgroups32 */ > + case __NR_setgroups: > + result = INTERNAL_SYSCALL_NCS (__NR_setgroups, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + break; > + default: > + result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3, cmdp->id[0], > + cmdp->id[1], cmdp->id[2]); > + } > int error = 0; > if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (result))) > { > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall 2023-04-24 15:03 [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott ` (2 preceding siblings ...) 2023-04-24 15:03 ` [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott @ 2023-05-08 14:13 ` Joe Simmons-Talbott 3 siblings, 0 replies; 22+ messages in thread From: Joe Simmons-Talbott @ 2023-05-08 14:13 UTC (permalink / raw) To: libc-alpha; +Cc: H.J. Lu, Szabolcs Nagy Is there anything further I need to do to get this patchset reviewed? Thanks, Joe On Mon, Apr 24, 2023 at 11:03:50AM -0400, Joe Simmons-Talbott wrote: > To make binary call tree analysis easier place the syscall number into > the register just before the syscall is made. Only do this if the > syscall number is a constant integer. > > Compiler optimizations can place quite a few instructions between the > setting of the syscall number and the syscall instruction. During call > tree analysis the number of instructions between the two can lead to > more difficulty for both tools and humans in properly identifying the > syscall number. Having the syscall number set in the prior instruction > to the syscall instruction makes this task easier and less error prone. > Being able to reliably identify syscalls made by a given API will make > it easier to understand and verify the safety and security of glibc. > > Tested on x86_64 and aarch64. > > Chnages to v5: > * aarch64: Remove '\n\t' from asm. > * x86_64: Remove unused macros. > > Changes to v4: > * aarch64: Move generated comment. Remove unused macros from earlier > version. > > * Added patch > - nptl: Convert indirect syscall numbers to compile time numeric > values. > > Changes to v3: > * x86_64: Combine two inline asms into one. > - Suggested by "H.J. Lu" <hjl.tools@gmail.com> > > Changes to v2: > * Add a more detailed commit message. > > Changes to v1: > * aarch64: Combine two inline asms into one. Avoid loading name twice. > > Joe Simmons-Talbott (3): > x86_64: Set the syscall register right before doing the syscall. > aarch64: Set the syscall register right before doing the syscall. > nptl: Use direct syscall numbers in setxid > > nptl/nptl_setxid.c | 136 ++++++++++++++++++++++- > sysdeps/unix/sysv/linux/aarch64/sysdep.h | 29 +++-- > sysdeps/unix/sysv/linux/x86_64/sysdep.h | 24 ++-- > 3 files changed, 164 insertions(+), 25 deletions(-) > > -- > 2.39.2 > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2023-06-28 19:17 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-24 15:03 [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott 2023-05-15 14:15 ` Joe Simmons-Talbott 2023-05-15 16:20 ` H.J. Lu 2023-05-25 18:07 ` Joe Simmons-Talbott 2023-05-25 18:40 ` Noah Goldstein 2023-05-26 7:04 ` Florian Weimer 2023-05-26 12:59 ` Joe Simmons-Talbott 2023-05-26 21:18 ` Noah Goldstein 2023-05-30 10:13 ` Florian Weimer 2023-05-31 18:23 ` Noah Goldstein 2023-06-28 19:17 ` Joe Simmons-Talbott 2023-04-24 15:03 ` [PATCH v6 2/3] aarch64: " Joe Simmons-Talbott 2023-05-09 7:47 ` Szabolcs Nagy 2023-04-24 15:03 ` [PATCH v6 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott 2023-04-24 15:17 ` Xi Ruoyao 2023-04-26 9:46 ` Szabolcs Nagy 2023-04-28 10:52 ` Florian Weimer 2023-04-26 12:39 ` Cristian Rodríguez 2023-04-26 13:24 ` Szabolcs Nagy 2023-05-25 18:07 ` Joe Simmons-Talbott 2023-05-08 14:13 ` [PATCH v6 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
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).