public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] x86_64: aarch64: Set call number just before syscall
@ 2023-04-11 13:30 Joe Simmons-Talbott
  2023-04-11 13:30 ` [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
                   ` (5 more replies)
  0 siblings, 6 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-11 13:30 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.

Tested on x86_64 and aarch64.

Joe Simmons-Talbott (2):
  x86_64: Set the syscall register right before doing the syscall.
  aarch64: Set the syscall register right before doing the syscall.

 sysdeps/unix/sysv/linux/aarch64/sysdep.h |  6 +++++
 sysdeps/unix/sysv/linux/x86_64/sysdep.h  | 33 ++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-11 13:30 ` Joe Simmons-Talbott
  2023-04-17 22:35   ` Noah Goldstein
  2023-04-11 13:30 ` [PATCH 2/2] aarch64: " Joe Simmons-Talbott
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-11 13:30 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.
---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h | 33 +++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index cfb51be8c5..800a56723f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -250,12 +250,20 @@
     (long int) resultvar;						\
 })
 
+#define MSTR_HELPER(x) #x
+#define MSTR(x) MSTR_HELPER(x)
+
 #undef internal_syscall1
 #define internal_syscall1(number, arg1)					\
 ({									\
     unsigned long int resultvar;					\
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -272,6 +280,11 @@
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -290,6 +303,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -310,6 +328,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -332,6 +355,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -357,6 +385,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-11 13:30 ` [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-11 13:30 ` Joe Simmons-Talbott
  2023-04-11 13:50   ` Florian Weimer
  2023-04-12 21:11 ` [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-11 13:30 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.
---
 sysdeps/unix/sysv/linux/aarch64/sysdep.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
index e94d1703ad..42a70feac2 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
@@ -167,12 +167,18 @@
 
 # define HAVE_CLONE3_WRAPPER		1
 
+# define MSTR_HELPER(x) # x
+# define MSTR(x) MSTR_HELPER(x)
+
 # undef INTERNAL_SYSCALL_RAW
 # define INTERNAL_SYSCALL_RAW(name, nr, args...)		\
   ({ long _sys_result;						\
      {								\
        LOAD_ARGS_##nr (args)					\
        register long _x8 asm ("x8") = (name);			\
+       if (__builtin_constant_p(name))				\
+         asm volatile ("mov	x8, " MSTR(name) ";"		\
+                       : /* no output */ : "i" (name) : "x8");	\
        asm volatile ("svc	0	// syscall " # name     \
 		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
        _sys_result = _x0;					\
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 13:30 ` [PATCH 2/2] aarch64: " Joe Simmons-Talbott
@ 2023-04-11 13:50   ` Florian Weimer
  2023-04-11 14:15     ` Adhemerval Zanella Netto
  0 siblings, 1 reply; 33+ messages in thread
From: Florian Weimer @ 2023-04-11 13:50 UTC (permalink / raw)
  To: Joe Simmons-Talbott via Libc-alpha; +Cc: Joe Simmons-Talbott

* Joe Simmons-Talbott via Libc-alpha:

>    ({ long _sys_result;						\
>       {								\
>         LOAD_ARGS_##nr (args)					\
>         register long _x8 asm ("x8") = (name);			\
> +       if (__builtin_constant_p(name))				\
> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
> +                       : /* no output */ : "i" (name) : "x8");	\
>         asm volatile ("svc	0	// syscall " # name     \
>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
>         _sys_result = _x0;					\

I think you should do this in a single assembler statement, load the
constant only once.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 13:50   ` Florian Weimer
@ 2023-04-11 14:15     ` Adhemerval Zanella Netto
  2023-04-11 15:43       ` Szabolcs Nagy
  2023-04-11 16:03       ` Florian Weimer
  0 siblings, 2 replies; 33+ messages in thread
From: Adhemerval Zanella Netto @ 2023-04-11 14:15 UTC (permalink / raw)
  To: Florian Weimer, Joe Simmons-Talbott via Libc-alpha; +Cc: Joe Simmons-Talbott



On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
> * Joe Simmons-Talbott via Libc-alpha:
> 
>>    ({ long _sys_result;						\
>>       {								\
>>         LOAD_ARGS_##nr (args)					\
>>         register long _x8 asm ("x8") = (name);			\
>> +       if (__builtin_constant_p(name))				\
>> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
>> +                       : /* no output */ : "i" (name) : "x8");	\
>>         asm volatile ("svc	0	// syscall " # name     \
>>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
>>         _sys_result = _x0;					\
> 
> I think you should do this in a single assembler statement, load the
> constant only once.

Is this required because compiler is free to reorganize the argument
list? I think it should me it clear on the commit message.

Using a single assembler would require two inline asm, something like:

diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h                                                                                            index e94d1703ad..2a128bb72d 100644                                                                                                                                                         --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h                                                                                                                                              +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h                                                                                                                                              @@ -172,9 +172,19 @@
   ({ 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"); \
+       if (__builtin_constant_p (name))                                \
+        asm volatile ("mov     x8, %1\n"                       \
+                      "svc     0       // syscall " # name     \
+                      : "=r" (_x0)                             \
+                      : "i" (name) ASM_ARGS_##nr               \
+                      : "x8", "memory");                       \
+       else                                                    \
+         {                                                     \
+          register long _x8 asm ("x8") = (name);               \
+          asm volatile ("svc      0       // syscall " # name  \
+                        : "=r" (_x0)                           \
+                        : "r"(_x8) ASM_ARGS_##nr : "memory");  \
+        }                                                      \
        _sys_result = _x0;                                      \
      }                                                         \
      _sys_result; })

Which really makes me doubt if this extra complexity is really necessary...


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 14:15     ` Adhemerval Zanella Netto
@ 2023-04-11 15:43       ` Szabolcs Nagy
  2023-04-11 16:03       ` Florian Weimer
  1 sibling, 0 replies; 33+ messages in thread
From: Szabolcs Nagy @ 2023-04-11 15:43 UTC (permalink / raw)
  To: Adhemerval Zanella Netto, Florian Weimer,
	Joe Simmons-Talbott via Libc-alpha
  Cc: Joe Simmons-Talbott

The 04/11/2023 11:15, Adhemerval Zanella Netto via Libc-alpha wrote:
> On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
> > * Joe Simmons-Talbott via Libc-alpha:
> > 
> >>    ({ long _sys_result;						\
> >>       {								\
> >>         LOAD_ARGS_##nr (args)					\
> >>         register long _x8 asm ("x8") = (name);			\
> >> +       if (__builtin_constant_p(name))				\
> >> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
> >> +                       : /* no output */ : "i" (name) : "x8");	\
> >>         asm volatile ("svc	0	// syscall " # name     \
> >>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
> >>         _sys_result = _x0;					\
> > 
> > I think you should do this in a single assembler statement, load the
> > constant only once.
> 
> Is this required because compiler is free to reorganize the argument
> list? I think it should me it clear on the commit message.
> 
> Using a single assembler would require two inline asm, something like:

i don't like the use of __builtin_constant_name_p here.
if we want different behaviour for INTERNAL_SYSCALL_NCS
and other inlined syscall things then we should do the
dispatch there.

and yes it has to be one asm block since the compiler
is free to add random nops (or equivalent) anywhere.

i'm not entirely sure why this change is needed, a bit
more background would be useful.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 14:15     ` Adhemerval Zanella Netto
  2023-04-11 15:43       ` Szabolcs Nagy
@ 2023-04-11 16:03       ` Florian Weimer
  2023-04-11 16:39         ` Adhemerval Zanella Netto
  1 sibling, 1 reply; 33+ messages in thread
From: Florian Weimer @ 2023-04-11 16:03 UTC (permalink / raw)
  To: Adhemerval Zanella Netto
  Cc: Joe Simmons-Talbott via Libc-alpha, Joe Simmons-Talbott

* Adhemerval Zanella Netto:

> On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
>> * Joe Simmons-Talbott via Libc-alpha:
>> 
>>>    ({ long _sys_result;						\
>>>       {								\
>>>         LOAD_ARGS_##nr (args)					\
>>>         register long _x8 asm ("x8") = (name);			\
>>> +       if (__builtin_constant_p(name))				\
>>> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
>>> +                       : /* no output */ : "i" (name) : "x8");	\
>>>         asm volatile ("svc	0	// syscall " # name     \
>>>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
>>>         _sys_result = _x0;					\
>> 
>> I think you should do this in a single assembler statement, load the
>> constant only once.
>
> Is this required because compiler is free to reorganize the argument
> list? I think it should me it clear on the commit message.

Yes, that's the reason.  It's a bit tricky to recover the system call
number using static analysis otherwise.  I suggested to Joe that we
should put something into glibc, rather than improving that static
analysis tool so that it's fully reliable.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 16:03       ` Florian Weimer
@ 2023-04-11 16:39         ` Adhemerval Zanella Netto
  2023-04-12 15:27           ` Joe Simmons-Talbott
  0 siblings, 1 reply; 33+ messages in thread
From: Adhemerval Zanella Netto @ 2023-04-11 16:39 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Joe Simmons-Talbott via Libc-alpha, Joe Simmons-Talbott



On 11/04/23 13:03, Florian Weimer wrote:
> * Adhemerval Zanella Netto:
> 
>> On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
>>> * Joe Simmons-Talbott via Libc-alpha:
>>>
>>>>    ({ long _sys_result;						\
>>>>       {								\
>>>>         LOAD_ARGS_##nr (args)					\
>>>>         register long _x8 asm ("x8") = (name);			\
>>>> +       if (__builtin_constant_p(name))				\
>>>> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
>>>> +                       : /* no output */ : "i" (name) : "x8");	\
>>>>         asm volatile ("svc	0	// syscall " # name     \
>>>>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
>>>>         _sys_result = _x0;					\
>>>
>>> I think you should do this in a single assembler statement, load the
>>> constant only once.
>>
>> Is this required because compiler is free to reorganize the argument
>> list? I think it should me it clear on the commit message.
> 
> Yes, that's the reason.  It's a bit tricky to recover the system call
> number using static analysis otherwise.  I suggested to Joe that we
> should put something into glibc, rather than improving that static
> analysis tool so that it's fully reliable.

Direct syscalls are done by different projects, like sanitizer, libgomp,
etc; so imho improving the static analysis tool could potentially catch
a wide range of usages than trying to fix only on glibc.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-11 16:39         ` Adhemerval Zanella Netto
@ 2023-04-12 15:27           ` Joe Simmons-Talbott
  0 siblings, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-12 15:27 UTC (permalink / raw)
  To: Adhemerval Zanella Netto
  Cc: Florian Weimer, Joe Simmons-Talbott via Libc-alpha

On Tue, Apr 11, 2023 at 01:39:31PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 11/04/23 13:03, Florian Weimer wrote:
> > * Adhemerval Zanella Netto:
> > 
> >> On 11/04/23 10:50, Florian Weimer via Libc-alpha wrote:
> >>> * Joe Simmons-Talbott via Libc-alpha:
> >>>
> >>>>    ({ long _sys_result;						\
> >>>>       {								\
> >>>>         LOAD_ARGS_##nr (args)					\
> >>>>         register long _x8 asm ("x8") = (name);			\
> >>>> +       if (__builtin_constant_p(name))				\
> >>>> +         asm volatile ("mov	x8, " MSTR(name) ";"		\
> >>>> +                       : /* no output */ : "i" (name) : "x8");	\
> >>>>         asm volatile ("svc	0	// syscall " # name     \
> >>>>  		     : "=r" (_x0) : "r"(_x8) ASM_ARGS_##nr : "memory");	\
> >>>>         _sys_result = _x0;					\
> >>>
> >>> I think you should do this in a single assembler statement, load the
> >>> constant only once.
> >>
> >> Is this required because compiler is free to reorganize the argument
> >> list? I think it should me it clear on the commit message.
> > 
> > Yes, that's the reason.  It's a bit tricky to recover the system call
> > number using static analysis otherwise.  I suggested to Joe that we
> > should put something into glibc, rather than improving that static
> > analysis tool so that it's fully reliable.
> 
> Direct syscalls are done by different projects, like sanitizer, libgomp,
> etc; so imho improving the static analysis tool could potentially catch
> a wide range of usages than trying to fix only on glibc.
> 

I agree that improving the static analysis tool would be helpful.  One
thing to keep in mind is that this patchset will also aid people doing
static analysis manually.  It also seems likely that other static
analysis tools would benefit from this change.

Thanks,
Joe


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-11 13:30 ` [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
  2023-04-11 13:30 ` [PATCH 2/2] aarch64: " Joe Simmons-Talbott
@ 2023-04-12 21:11 ` Joe Simmons-Talbott
  2023-04-12 21:11   ` [PATCH v2 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
  2023-04-12 21:11   ` [PATCH v2 2/2] aarch64: " Joe Simmons-Talbott
  2023-04-17 15:34 ` [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-12 21:11 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.

Tested on x86_64 and aarch64.

Changes to v1:
  * aarch64: Combine two inline asms into one.  Avoid loading name twice.

Joe Simmons-Talbott (2):
  x86_64: Set the syscall register right before doing the syscall.
  aarch64: Set the syscall register right before doing the syscall.

 sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h  | 33 ++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-12 21:11 ` [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-12 21:11   ` Joe Simmons-Talbott
  2023-04-12 21:11   ` [PATCH v2 2/2] aarch64: " Joe Simmons-Talbott
  1 sibling, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-12 21:11 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.
---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h | 33 +++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index cfb51be8c5..800a56723f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -250,12 +250,20 @@
     (long int) resultvar;						\
 })
 
+#define MSTR_HELPER(x) #x
+#define MSTR(x) MSTR_HELPER(x)
+
 #undef internal_syscall1
 #define internal_syscall1(number, arg1)					\
 ({									\
     unsigned long int resultvar;					\
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -272,6 +280,11 @@
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -290,6 +303,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -310,6 +328,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -332,6 +355,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -357,6 +385,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-12 21:11 ` [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-12 21:11   ` [PATCH v2 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-12 21:11   ` Joe Simmons-Talbott
  1 sibling, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-12 21:11 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.
---
 sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
index e94d1703ad..b91656fdff 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
@@ -167,14 +167,28 @@
 
 # define HAVE_CLONE3_WRAPPER		1
 
+# define MSTR_HELPER(x) # x
+# define MSTR(x) MSTR_HELPER(x)
+
 # 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");	\
+       if (__builtin_constant_p(name))				\
+         asm volatile ("mov	x8, %1\n"			\
+		       "svc	0	// syscall " # name	\
+                       : "=r" (_x0)				\
+		       : "i" (name) ASM_ARGS_##nr		\
+		       : "x8", "memory");			\
+       else							\
+         {							\
+           register long _x8 asm ("x8") = (name);		\
+           asm volatile ("svc	0	// syscall " # name     \
+		         : "=r" (_x0)				\
+		         : "r"(_x8) ASM_ARGS_##nr		\
+		         : "memory");				\
+         }							\
        _sys_result = _x0;					\
      }								\
      _sys_result; })
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
                   ` (2 preceding siblings ...)
  2023-04-12 21:11 ` [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-17 15:34 ` Joe Simmons-Talbott
  2023-04-17 15:34   ` [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
  2023-04-17 15:34   ` [PATCH v3 2/2] aarch64: " Joe Simmons-Talbott
  2023-04-17 21:20 ` [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  5 siblings, 2 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 15:34 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.

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 (2):
  x86_64: Set the syscall register right before doing the syscall.
  aarch64: Set the syscall register right before doing the syscall.

 sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h  | 33 ++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 3 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 15:34 ` [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-17 15:34   ` Joe Simmons-Talbott
  2023-04-17 15:54     ` H.J. Lu
  2023-04-17 15:34   ` [PATCH v3 2/2] aarch64: " Joe Simmons-Talbott
  1 sibling, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 15:34 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 | 33 +++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index cfb51be8c5..800a56723f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -250,12 +250,20 @@
     (long int) resultvar;						\
 })
 
+#define MSTR_HELPER(x) #x
+#define MSTR(x) MSTR_HELPER(x)
+
 #undef internal_syscall1
 #define internal_syscall1(number, arg1)					\
 ({									\
     unsigned long int resultvar;					\
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -272,6 +280,11 @@
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -290,6 +303,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -310,6 +328,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -332,6 +355,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
@@ -357,6 +385,11 @@
     register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;			\
     register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;			\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
+    if (__builtin_constant_p(number))                        \
+      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
+        : /* no outputs */                                   \
+        : "i" (number)                                       \
+		: "eax");                                            \
     asm volatile (							\
     "syscall\n\t"							\
     : "=a" (resultvar)							\
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-17 15:34 ` [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-17 15:34   ` [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-17 15:34   ` Joe Simmons-Talbott
  1 sibling, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 15:34 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 | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
index e94d1703ad..b91656fdff 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
@@ -167,14 +167,28 @@
 
 # define HAVE_CLONE3_WRAPPER		1
 
+# define MSTR_HELPER(x) # x
+# define MSTR(x) MSTR_HELPER(x)
+
 # 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");	\
+       if (__builtin_constant_p(name))				\
+         asm volatile ("mov	x8, %1\n"			\
+		       "svc	0	// syscall " # name	\
+                       : "=r" (_x0)				\
+		       : "i" (name) ASM_ARGS_##nr		\
+		       : "x8", "memory");			\
+       else							\
+         {							\
+           register long _x8 asm ("x8") = (name);		\
+           asm volatile ("svc	0	// syscall " # name     \
+		         : "=r" (_x0)				\
+		         : "r"(_x8) ASM_ARGS_##nr		\
+		         : "memory");				\
+         }							\
        _sys_result = _x0;					\
      }								\
      _sys_result; })
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 15:34   ` [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-17 15:54     ` H.J. Lu
  2023-04-17 16:00       ` Joe Simmons-Talbott
  0 siblings, 1 reply; 33+ messages in thread
From: H.J. Lu @ 2023-04-17 15:54 UTC (permalink / raw)
  To: Joe Simmons-Talbott; +Cc: libc-alpha

On Mon, Apr 17, 2023 at 8:35 AM Joe Simmons-Talbott via Libc-alpha
<libc-alpha@sourceware.org> 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 | 33 +++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> index cfb51be8c5..800a56723f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> @@ -250,12 +250,20 @@
>      (long int) resultvar;                                              \
>  })
>
> +#define MSTR_HELPER(x) #x
> +#define MSTR(x) MSTR_HELPER(x)
> +
>  #undef internal_syscall1
>  #define internal_syscall1(number, arg1)                                        \
>  ({                                                                     \
>      unsigned long int resultvar;                                       \
>      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -272,6 +280,11 @@
>      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -290,6 +303,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -310,6 +328,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -332,6 +355,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -357,6 +385,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> --
> 2.39.2
>

Won't the compiler load EAX twice when number is a constant?

-- 
H.J.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 15:54     ` H.J. Lu
@ 2023-04-17 16:00       ` Joe Simmons-Talbott
  2023-04-17 18:38         ` H.J. Lu
  0 siblings, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 16:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha

On Mon, Apr 17, 2023 at 08:54:17AM -0700, H.J. Lu wrote:
> On Mon, Apr 17, 2023 at 8:35 AM Joe Simmons-Talbott via Libc-alpha
> <libc-alpha@sourceware.org> 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 | 33 +++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > index cfb51be8c5..800a56723f 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > @@ -250,12 +250,20 @@
> >      (long int) resultvar;                                              \
> >  })
> >
> > +#define MSTR_HELPER(x) #x
> > +#define MSTR(x) MSTR_HELPER(x)
> > +
> >  #undef internal_syscall1
> >  #define internal_syscall1(number, arg1)                                        \
> >  ({                                                                     \
> >      unsigned long int resultvar;                                       \
> >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -272,6 +280,11 @@
> >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -290,6 +303,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -310,6 +328,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -332,6 +355,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -357,6 +385,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > --
> > 2.39.2
> >
> 
> Won't the compiler load EAX twice when number is a constant?
> 

Yes.  I'll have a new version combining the two asm sections into one
like for aarch64 soon.

Joe


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 16:00       ` Joe Simmons-Talbott
@ 2023-04-17 18:38         ` H.J. Lu
  0 siblings, 0 replies; 33+ messages in thread
From: H.J. Lu @ 2023-04-17 18:38 UTC (permalink / raw)
  To: Joe Simmons-Talbott; +Cc: libc-alpha

[-- Attachment #1: Type: text/plain, Size: 7331 bytes --]

On Mon, Apr 17, 2023 at 9:00 AM Joe Simmons-Talbott <josimmon@redhat.com> wrote:
>
> On Mon, Apr 17, 2023 at 08:54:17AM -0700, H.J. Lu wrote:
> > On Mon, Apr 17, 2023 at 8:35 AM Joe Simmons-Talbott via Libc-alpha
> > <libc-alpha@sourceware.org> 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 | 33 +++++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > >
> > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > > index cfb51be8c5..800a56723f 100644
> > > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > > @@ -250,12 +250,20 @@
> > >      (long int) resultvar;                                              \
> > >  })
> > >
> > > +#define MSTR_HELPER(x) #x
> > > +#define MSTR(x) MSTR_HELPER(x)
> > > +
> > >  #undef internal_syscall1
> > >  #define internal_syscall1(number, arg1)                                        \
> > >  ({                                                                     \
> > >      unsigned long int resultvar;                                       \
> > >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > @@ -272,6 +280,11 @@
> > >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> > >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > @@ -290,6 +303,11 @@
> > >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> > >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > @@ -310,6 +328,11 @@
> > >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> > >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > @@ -332,6 +355,11 @@
> > >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> > >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > @@ -357,6 +385,11 @@
> > >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> > >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> > >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > > +    if (__builtin_constant_p(number))                        \
> > > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > > +        : /* no outputs */                                   \
> > > +        : "i" (number)                                       \
> > > +               : "eax");                                            \
> > >      asm volatile (                                                     \
> > >      "syscall\n\t"                                                      \
> > >      : "=a" (resultvar)                                                 \
> > > --
> > > 2.39.2
> > >
> >
> > Won't the compiler load EAX twice when number is a constant?
> >
>
> Yes.  I'll have a new version combining the two asm sections into one
> like for aarch64 soon.
>
> Joe
>

There is no need for __builtin_constant_p.  Please try this.

-- 
H.J.

[-- Attachment #2: syscall.patch --]
[-- Type: text/x-patch, Size: 4217 bytes --]

diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index cfb51be8c5..58975bdf3f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -243,9 +243,9 @@
 ({									\
     unsigned long int resultvar;					\
     asm volatile (							\
-    "syscall\n\t"							\
+    "mov %1, %k0\n\tsyscall\n\t"					\
     : "=a" (resultvar)							\
-    : "0" (number)							\
+    : "g" (number)							\
     : "memory", REGISTERS_CLOBBERED_BY_SYSCALL);			\
     (long int) resultvar;						\
 })
@@ -257,9 +257,9 @@
     TYPEFY (arg1, __arg1) = ARGIFY (arg1);			 	\
     register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;			\
     asm volatile (							\
-    "syscall\n\t"							\
+    "mov %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"							\
+    "mov %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"							\
+    "mov %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"							\
+    "mov %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"							\
+    "mov %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"							\
+    "mov %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;						\
diff --git a/sysdeps/x86_64/nptl/tls.h b/sysdeps/x86_64/nptl/tls.h
index 1403f939f7..f206b11295 100644
--- a/sysdeps/x86_64/nptl/tls.h
+++ b/sysdeps/x86_64/nptl/tls.h
@@ -149,9 +149,9 @@ _Static_assert (offsetof (tcbhead_t, __glibc_unused2) == 0x80,
      _head->self = _thrdescr;						      \
 									      \
      /* It is a simple syscall to set the %fs value for the thread.  */	      \
-     asm volatile ("syscall"						      \
+     asm volatile ("mov %1, %k0\n\tsyscall"				      \
 		   : "=a" (_result)					      \
-		   : "0" ((unsigned long int) __NR_arch_prctl),		      \
+		   : "i" (__NR_arch_prctl),				      \
 		     "D" ((unsigned long int) ARCH_SET_FS),		      \
 		     "S" (_thrdescr)					      \
 		   : "memory", "cc", "r11", "cx");			      \

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
                   ` (3 preceding siblings ...)
  2023-04-17 15:34 ` [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-17 21:20 ` Joe Simmons-Talbott
  2023-04-17 21:20   ` [PATCH v4 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
  2023-04-17 21:20   ` [PATCH v4 2/2] aarch64: " Joe Simmons-Talbott
  2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  5 siblings, 2 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 21:20 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.

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 (2):
  x86_64: Set the syscall register right before doing the syscall.
  aarch64: Set the syscall register right before doing the syscall.

 sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++++++---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h  | 27 +++++++++++++-----------
 2 files changed, 32 insertions(+), 15 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v4 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 21:20 ` [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-17 21:20   ` Joe Simmons-Talbott
  2023-04-17 21:20   ` [PATCH v4 2/2] aarch64: " Joe Simmons-Talbott
  1 sibling, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 21:20 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 | 27 ++++++++++++++-----------
 1 file changed, 15 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..fd9eb4b02f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -250,6 +250,9 @@
     (long int) resultvar;						\
 })
 
+#define MSTR_HELPER(x) #x
+#define MSTR(x) MSTR_HELPER(x)
+
 #undef internal_syscall1
 #define internal_syscall1(number, arg1)					\
 ({									\
@@ -257,9 +260,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 +276,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 +294,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 +314,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 +336,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 +361,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] 33+ messages in thread

* [PATCH v4 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-17 21:20 ` [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-17 21:20   ` [PATCH v4 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-17 21:20   ` Joe Simmons-Talbott
  2023-04-18 12:57     ` Szabolcs Nagy
  1 sibling, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-17 21:20 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 | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
index e94d1703ad..b91656fdff 100644
--- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
@@ -167,14 +167,28 @@
 
 # define HAVE_CLONE3_WRAPPER		1
 
+# define MSTR_HELPER(x) # x
+# define MSTR(x) MSTR_HELPER(x)
+
 # 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");	\
+       if (__builtin_constant_p(name))				\
+         asm volatile ("mov	x8, %1\n"			\
+		       "svc	0	// syscall " # name	\
+                       : "=r" (_x0)				\
+		       : "i" (name) ASM_ARGS_##nr		\
+		       : "x8", "memory");			\
+       else							\
+         {							\
+           register long _x8 asm ("x8") = (name);		\
+           asm volatile ("svc	0	// syscall " # name     \
+		         : "=r" (_x0)				\
+		         : "r"(_x8) ASM_ARGS_##nr		\
+		         : "memory");				\
+         }							\
        _sys_result = _x0;					\
      }								\
      _sys_result; })
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-11 13:30 ` [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-17 22:35   ` Noah Goldstein
  2023-04-17 22:36     ` Noah Goldstein
  0 siblings, 1 reply; 33+ messages in thread
From: Noah Goldstein @ 2023-04-17 22:35 UTC (permalink / raw)
  To: Joe Simmons-Talbott; +Cc: GNU C Library

[-- Attachment #1: Type: text/plain, Size: 5974 bytes --]

On Tue, Apr 11, 2023 at 8:32 AM Joe Simmons-Talbott via Libc-alpha <
libc-alpha@sourceware.org> wrote:
>
> To make identifying syscalls easier during call tree analysis load the
> syscall number just before performing the syscall.
> ---
>  sysdeps/unix/sysv/linux/x86_64/sysdep.h | 33 +++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> index cfb51be8c5..800a56723f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> @@ -250,12 +250,20 @@
>      (long int) resultvar;                                              \
>  })
>
> +#define MSTR_HELPER(x) #x
> +#define MSTR(x) MSTR_HELPER(x)
> +
>  #undef internal_syscall1
>  #define internal_syscall1(number, arg1)
      \
>  ({                                                                     \
>      unsigned long int resultvar;                                       \
>      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -272,6 +280,11 @@
>      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
Is it ever possible for another instruction to be re-ordered between setting
`eax` and the `syscall`?
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -290,6 +303,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -310,6 +328,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -332,6 +355,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> @@ -357,6 +385,11 @@
>      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
>      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
>      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> +    if (__builtin_constant_p(number))                        \
> +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> +        : /* no outputs */                                   \
> +        : "i" (number)                                       \
> +               : "eax");                                            \
>      asm volatile (                                                     \
>      "syscall\n\t"                                                      \
>      : "=a" (resultvar)                                                 \
> --
> 2.39.2
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall.
  2023-04-17 22:35   ` Noah Goldstein
@ 2023-04-17 22:36     ` Noah Goldstein
  0 siblings, 0 replies; 33+ messages in thread
From: Noah Goldstein @ 2023-04-17 22:36 UTC (permalink / raw)
  To: Joe Simmons-Talbott; +Cc: GNU C Library

On Mon, Apr 17, 2023 at 5:35 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
>
>
> On Tue, Apr 11, 2023 at 8:32 AM Joe Simmons-Talbott via Libc-alpha <libc-alpha@sourceware.org> wrote:
> >
> > To make identifying syscalls easier during call tree analysis load the
> > syscall number just before performing the syscall.
> > ---
> >  sysdeps/unix/sysv/linux/x86_64/sysdep.h | 33 +++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > index cfb51be8c5..800a56723f 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > @@ -250,12 +250,20 @@
> >      (long int) resultvar;                                              \
> >  })
> >
> > +#define MSTR_HELPER(x) #x
> > +#define MSTR(x) MSTR_HELPER(x)
> > +
> >  #undef internal_syscall1
> >  #define internal_syscall1(number, arg1)                                        \
> >  ({                                                                     \
> >      unsigned long int resultvar;                                       \
> >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -272,6 +280,11 @@
> >      TYPEFY (arg1, __arg1) = ARGIFY (arg1);                             \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> Is it ever possible for another instruction to be re-ordered between setting
> `eax` and the `syscall`?

nevermind, you addressed in V2.
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -290,6 +303,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -310,6 +328,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -332,6 +355,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > @@ -357,6 +385,11 @@
> >      register TYPEFY (arg3, _a3) asm ("rdx") = __arg3;                  \
> >      register TYPEFY (arg2, _a2) asm ("rsi") = __arg2;                  \
> >      register TYPEFY (arg1, _a1) asm ("rdi") = __arg1;                  \
> > +    if (__builtin_constant_p(number))                        \
> > +      asm volatile ("movl $" MSTR(number) ", %%eax\n\t" \
> > +        : /* no outputs */                                   \
> > +        : "i" (number)                                       \
> > +               : "eax");                                            \
> >      asm volatile (                                                     \
> >      "syscall\n\t"                                                      \
> >      : "=a" (resultvar)                                                 \
> > --
> > 2.39.2
> >

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-17 21:20   ` [PATCH v4 2/2] aarch64: " Joe Simmons-Talbott
@ 2023-04-18 12:57     ` Szabolcs Nagy
  2023-04-18 19:33       ` Joe Simmons-Talbott
  0 siblings, 1 reply; 33+ messages in thread
From: Szabolcs Nagy @ 2023-04-18 12:57 UTC (permalink / raw)
  To: Joe Simmons-Talbott, libc-alpha

The 04/17/2023 17:20, 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.

since the code has !__builtin_constant_p(name) case
how would that be handled by these tools?

> ---
>  sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> index e94d1703ad..b91656fdff 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> @@ -167,14 +167,28 @@
>  
>  # define HAVE_CLONE3_WRAPPER		1
>  
> +# define MSTR_HELPER(x) # x
> +# define MSTR(x) MSTR_HELPER(x)
> +

i dont see this used.

>  # 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");	\
> +       if (__builtin_constant_p(name))				\
> +         asm volatile ("mov	x8, %1\n"			\
> +		       "svc	0	// syscall " # name	\
> +                       : "=r" (_x0)				\
> +		       : "i" (name) ASM_ARGS_##nr		\
> +		       : "x8", "memory");			\
> +       else							\
> +         {							\
> +           register long _x8 asm ("x8") = (name);		\
> +           asm volatile ("svc	0	// syscall " # name     \
> +		         : "=r" (_x0)				\
> +		         : "r"(_x8) ASM_ARGS_##nr		\
> +		         : "memory");				\
> +         }							\
>         _sys_result = _x0;					\
>       }								\
>       _sys_result; })

i guess this is ok.

i would probably move the generated comment to the
mov x8,%1 line and remove it in the non-const case.
but i rarely look at compiler output..

it seems the only cases when the name is non-const are

nptl/nptl_setxid.c:  result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0],
nptl/nptl_setxid.c:  result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3,

which in turn only happens for syscalls

sysdeps/unix/sysv/linux/setuid.c:  return INLINE_SETXID_SYSCALL (setuid32, 1, uid);
sysdeps/unix/sysv/linux/setuid.c:  return INLINE_SETXID_SYSCALL (setuid, 1, uid);
sysdeps/unix/sysv/linux/setreuid.c:  return INLINE_SETXID_SYSCALL (setreuid32, 2, ruid, euid);
sysdeps/unix/sysv/linux/setreuid.c:  return INLINE_SETXID_SYSCALL (setreuid, 2, ruid, euid);
sysdeps/unix/sysv/linux/setegid.c:  result = INLINE_SETXID_SYSCALL (setresgid32, 3, -1, gid, -1);
sysdeps/unix/sysv/linux/setegid.c:  result = INLINE_SETXID_SYSCALL (setresgid, 3, -1, gid, -1);
sysdeps/unix/sysv/linux/setregid.c:  return INLINE_SETXID_SYSCALL (setregid32, 2, rgid, egid);
sysdeps/unix/sysv/linux/setregid.c:  return INLINE_SETXID_SYSCALL (setregid, 2, rgid, egid);
sysdeps/unix/sysv/linux/setgid.c:  return INLINE_SETXID_SYSCALL (setgid32, 1, gid);
sysdeps/unix/sysv/linux/setgid.c:  return INLINE_SETXID_SYSCALL (setgid, 1, gid);
sysdeps/unix/sysv/linux/setgroups.c:  return INLINE_SETXID_SYSCALL (setgroups32, 2, n, groups);
sysdeps/unix/sysv/linux/setgroups.c:  return INLINE_SETXID_SYSCALL (setgroups, 2, n, groups);
sysdeps/unix/sysv/linux/setresgid.c:  return INLINE_SETXID_SYSCALL (setresgid32, 3, rgid, egid, sgid);
sysdeps/unix/sysv/linux/setresgid.c:  return INLINE_SETXID_SYSCALL (setresgid, 3, rgid, egid, sgid);
sysdeps/unix/sysv/linux/setresuid.c:  return INLINE_SETXID_SYSCALL (setresuid32, 3, ruid, euid, suid);
sysdeps/unix/sysv/linux/setresuid.c:  return INLINE_SETXID_SYSCALL (setresuid, 3, ruid, euid, suid);
sysdeps/unix/sysv/linux/seteuid.c:  result = INLINE_SETXID_SYSCALL (setresuid32, 3, -1, uid, -1);
sysdeps/unix/sysv/linux/seteuid.c:  result = INLINE_SETXID_SYSCALL (setresuid, 3, -1, uid, -1);

so if we wanted we could have a switch statement in setxid
and make all syscalls compile time const (other than
explicit external calls to syscall())

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v4 2/2] aarch64: Set the syscall register right before doing the syscall.
  2023-04-18 12:57     ` Szabolcs Nagy
@ 2023-04-18 19:33       ` Joe Simmons-Talbott
  0 siblings, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-18 19:33 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: libc-alpha

On Tue, Apr 18, 2023 at 01:57:28PM +0100, Szabolcs Nagy wrote:
> The 04/17/2023 17:20, 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.
> 
> since the code has !__builtin_constant_p(name) case
> how would that be handled by these tools?
> 
> > ---
> >  sysdeps/unix/sysv/linux/aarch64/sysdep.h | 20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/sysdep.h b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > index e94d1703ad..b91656fdff 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/aarch64/sysdep.h
> > @@ -167,14 +167,28 @@
> >  
> >  # define HAVE_CLONE3_WRAPPER		1
> >  
> > +# define MSTR_HELPER(x) # x
> > +# define MSTR(x) MSTR_HELPER(x)
> > +
> 
> i dont see this used.
> 
> >  # 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");	\
> > +       if (__builtin_constant_p(name))				\
> > +         asm volatile ("mov	x8, %1\n"			\
> > +		       "svc	0	// syscall " # name	\
> > +                       : "=r" (_x0)				\
> > +		       : "i" (name) ASM_ARGS_##nr		\
> > +		       : "x8", "memory");			\
> > +       else							\
> > +         {							\
> > +           register long _x8 asm ("x8") = (name);		\
> > +           asm volatile ("svc	0	// syscall " # name     \
> > +		         : "=r" (_x0)				\
> > +		         : "r"(_x8) ASM_ARGS_##nr		\
> > +		         : "memory");				\
> > +         }							\
> >         _sys_result = _x0;					\
> >       }								\
> >       _sys_result; })
> 
> i guess this is ok.
> 
> i would probably move the generated comment to the
> mov x8,%1 line and remove it in the non-const case.
> but i rarely look at compiler output..
> 
> it seems the only cases when the name is non-const are
> 
> nptl/nptl_setxid.c:  result = INTERNAL_SYSCALL_NCS (xidcmd->syscall_no, 3, xidcmd->id[0],
> nptl/nptl_setxid.c:  result = INTERNAL_SYSCALL_NCS (cmdp->syscall_no, 3,
> 
> which in turn only happens for syscalls
> 
> sysdeps/unix/sysv/linux/setuid.c:  return INLINE_SETXID_SYSCALL (setuid32, 1, uid);
> sysdeps/unix/sysv/linux/setuid.c:  return INLINE_SETXID_SYSCALL (setuid, 1, uid);
> sysdeps/unix/sysv/linux/setreuid.c:  return INLINE_SETXID_SYSCALL (setreuid32, 2, ruid, euid);
> sysdeps/unix/sysv/linux/setreuid.c:  return INLINE_SETXID_SYSCALL (setreuid, 2, ruid, euid);
> sysdeps/unix/sysv/linux/setegid.c:  result = INLINE_SETXID_SYSCALL (setresgid32, 3, -1, gid, -1);
> sysdeps/unix/sysv/linux/setegid.c:  result = INLINE_SETXID_SYSCALL (setresgid, 3, -1, gid, -1);
> sysdeps/unix/sysv/linux/setregid.c:  return INLINE_SETXID_SYSCALL (setregid32, 2, rgid, egid);
> sysdeps/unix/sysv/linux/setregid.c:  return INLINE_SETXID_SYSCALL (setregid, 2, rgid, egid);
> sysdeps/unix/sysv/linux/setgid.c:  return INLINE_SETXID_SYSCALL (setgid32, 1, gid);
> sysdeps/unix/sysv/linux/setgid.c:  return INLINE_SETXID_SYSCALL (setgid, 1, gid);
> sysdeps/unix/sysv/linux/setgroups.c:  return INLINE_SETXID_SYSCALL (setgroups32, 2, n, groups);
> sysdeps/unix/sysv/linux/setgroups.c:  return INLINE_SETXID_SYSCALL (setgroups, 2, n, groups);
> sysdeps/unix/sysv/linux/setresgid.c:  return INLINE_SETXID_SYSCALL (setresgid32, 3, rgid, egid, sgid);
> sysdeps/unix/sysv/linux/setresgid.c:  return INLINE_SETXID_SYSCALL (setresgid, 3, rgid, egid, sgid);
> sysdeps/unix/sysv/linux/setresuid.c:  return INLINE_SETXID_SYSCALL (setresuid32, 3, ruid, euid, suid);
> sysdeps/unix/sysv/linux/setresuid.c:  return INLINE_SETXID_SYSCALL (setresuid, 3, ruid, euid, suid);
> sysdeps/unix/sysv/linux/seteuid.c:  result = INLINE_SETXID_SYSCALL (setresuid32, 3, -1, uid, -1);
> sysdeps/unix/sysv/linux/seteuid.c:  result = INLINE_SETXID_SYSCALL (setresuid, 3, -1, uid, -1);
> 
> so if we wanted we could have a switch statement in setxid
> and make all syscalls compile time const (other than
> explicit external calls to syscall())
> 

If we go this route should I include #ifdef guards around each case or
just the 32 bit ones?

Thanks,
Joe


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall
  2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
                   ` (4 preceding siblings ...)
  2023-04-17 21:20 ` [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-19 13:58 ` Joe Simmons-Talbott
  2023-04-19 13:58   ` [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
                     ` (2 more replies)
  5 siblings, 3 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 13:58 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.

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  |  27 +++--
 3 files changed, 167 insertions(+), 25 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall.
  2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
@ 2023-04-19 13:58   ` Joe Simmons-Talbott
  2023-04-19 15:35     ` H.J. Lu
  2023-04-19 13:58   ` [PATCH v5 2/3] aarch64: " Joe Simmons-Talbott
  2023-04-19 13:58   ` [PATCH v5 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott
  2 siblings, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 13:58 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 | 27 ++++++++++++++-----------
 1 file changed, 15 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..fd9eb4b02f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -250,6 +250,9 @@
     (long int) resultvar;						\
 })
 
+#define MSTR_HELPER(x) #x
+#define MSTR(x) MSTR_HELPER(x)
+
 #undef internal_syscall1
 #define internal_syscall1(number, arg1)					\
 ({									\
@@ -257,9 +260,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 +276,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 +294,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 +314,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 +336,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 +361,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] 33+ messages in thread

* [PATCH v5 2/3] aarch64: Set the syscall register right before doing the syscall.
  2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-19 13:58   ` [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-19 13:58   ` Joe Simmons-Talbott
  2023-04-19 14:56     ` Szabolcs Nagy
  2023-04-19 13:58   ` [PATCH v5 3/3] nptl: Use direct syscall numbers in setxid Joe Simmons-Talbott
  2 siblings, 1 reply; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 13:58 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\n\t"					\
+		         : "=r" (_x0)					\
+		         : "r"(_x8) ASM_ARGS_##nr			\
+		         : "memory");					\
+         }								\
+       _sys_result = _x0;						\
+     }									\
      _sys_result; })
 
 # undef INTERNAL_SYSCALL
-- 
2.39.2


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v5 3/3] nptl: Use direct syscall numbers in setxid
  2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
  2023-04-19 13:58   ` [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
  2023-04-19 13:58   ` [PATCH v5 2/3] aarch64: " Joe Simmons-Talbott
@ 2023-04-19 13:58   ` Joe Simmons-Talbott
  2 siblings, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 13:58 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] 33+ messages in thread

* Re: [PATCH v5 2/3] aarch64: Set the syscall register right before doing the syscall.
  2023-04-19 13:58   ` [PATCH v5 2/3] aarch64: " Joe Simmons-Talbott
@ 2023-04-19 14:56     ` Szabolcs Nagy
  2023-04-19 15:21       ` Joe Simmons-Talbott
  0 siblings, 1 reply; 33+ messages in thread
From: Szabolcs Nagy @ 2023-04-19 14:56 UTC (permalink / raw)
  To: Joe Simmons-Talbott, libc-alpha

The 04/19/2023 09:58, Joe Simmons-Talbott via Libc-alpha wrote:
> --- 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\n\t"					\

why \n\t ?

i don't think that's needed.

> +		         : "=r" (_x0)					\
> +		         : "r"(_x8) ASM_ARGS_##nr			\
> +		         : "memory");					\
> +         }								\
> +       _sys_result = _x0;						\
> +     }									\
>       _sys_result; })
>  
>  # undef INTERNAL_SYSCALL

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 2/3] aarch64: Set the syscall register right before doing the syscall.
  2023-04-19 14:56     ` Szabolcs Nagy
@ 2023-04-19 15:21       ` Joe Simmons-Talbott
  0 siblings, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 15:21 UTC (permalink / raw)
  To: Szabolcs Nagy; +Cc: libc-alpha

On Wed, Apr 19, 2023 at 03:56:05PM +0100, Szabolcs Nagy wrote:
> The 04/19/2023 09:58, Joe Simmons-Talbott via Libc-alpha wrote:
> > --- 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\n\t"					\
> 
> why \n\t ?
> 
> i don't think that's needed.

I'll remove it in v6.

Thanks,
Joe


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall.
  2023-04-19 13:58   ` [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
@ 2023-04-19 15:35     ` H.J. Lu
  2023-04-19 15:48       ` Joe Simmons-Talbott
  0 siblings, 1 reply; 33+ messages in thread
From: H.J. Lu @ 2023-04-19 15:35 UTC (permalink / raw)
  To: Joe Simmons-Talbott; +Cc: libc-alpha

On Wed, Apr 19, 2023 at 6:59 AM Joe Simmons-Talbott via Libc-alpha
<libc-alpha@sourceware.org> 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 | 27 ++++++++++++++-----------
>  1 file changed, 15 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..fd9eb4b02f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> @@ -250,6 +250,9 @@
>      (long int) resultvar;                                              \
>  })
>
> +#define MSTR_HELPER(x) #x
> +#define MSTR(x) MSTR_HELPER(x)

These are unused.

>  #undef internal_syscall1
>  #define internal_syscall1(number, arg1)                                        \
>  ({                                                                     \
> @@ -257,9 +260,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 +276,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 +294,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 +314,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 +336,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 +361,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
>


-- 
H.J.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall.
  2023-04-19 15:35     ` H.J. Lu
@ 2023-04-19 15:48       ` Joe Simmons-Talbott
  0 siblings, 0 replies; 33+ messages in thread
From: Joe Simmons-Talbott @ 2023-04-19 15:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha

On Wed, Apr 19, 2023 at 08:35:30AM -0700, H.J. Lu wrote:
> On Wed, Apr 19, 2023 at 6:59 AM Joe Simmons-Talbott via Libc-alpha
> <libc-alpha@sourceware.org> 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 | 27 ++++++++++++++-----------
> >  1 file changed, 15 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..fd9eb4b02f 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > @@ -250,6 +250,9 @@
> >      (long int) resultvar;                                              \
> >  })
> >
> > +#define MSTR_HELPER(x) #x
> > +#define MSTR(x) MSTR_HELPER(x)
> 
> These are unused.
> 

I'll remove this in v6.

Thanks,
Joe


^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2023-04-19 15:48 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 13:30 [PATCH 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
2023-04-11 13:30 ` [PATCH 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
2023-04-17 22:35   ` Noah Goldstein
2023-04-17 22:36     ` Noah Goldstein
2023-04-11 13:30 ` [PATCH 2/2] aarch64: " Joe Simmons-Talbott
2023-04-11 13:50   ` Florian Weimer
2023-04-11 14:15     ` Adhemerval Zanella Netto
2023-04-11 15:43       ` Szabolcs Nagy
2023-04-11 16:03       ` Florian Weimer
2023-04-11 16:39         ` Adhemerval Zanella Netto
2023-04-12 15:27           ` Joe Simmons-Talbott
2023-04-12 21:11 ` [PATCH v2 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
2023-04-12 21:11   ` [PATCH v2 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
2023-04-12 21:11   ` [PATCH v2 2/2] aarch64: " Joe Simmons-Talbott
2023-04-17 15:34 ` [PATCH v3 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
2023-04-17 15:34   ` [PATCH v3 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
2023-04-17 15:54     ` H.J. Lu
2023-04-17 16:00       ` Joe Simmons-Talbott
2023-04-17 18:38         ` H.J. Lu
2023-04-17 15:34   ` [PATCH v3 2/2] aarch64: " Joe Simmons-Talbott
2023-04-17 21:20 ` [PATCH v4 0/2] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
2023-04-17 21:20   ` [PATCH v4 1/2] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
2023-04-17 21:20   ` [PATCH v4 2/2] aarch64: " Joe Simmons-Talbott
2023-04-18 12:57     ` Szabolcs Nagy
2023-04-18 19:33       ` Joe Simmons-Talbott
2023-04-19 13:58 ` [PATCH v5 0/3] x86_64: aarch64: Set call number just before syscall Joe Simmons-Talbott
2023-04-19 13:58   ` [PATCH v5 1/3] x86_64: Set the syscall register right before doing the syscall Joe Simmons-Talbott
2023-04-19 15:35     ` H.J. Lu
2023-04-19 15:48       ` Joe Simmons-Talbott
2023-04-19 13:58   ` [PATCH v5 2/3] aarch64: " Joe Simmons-Talbott
2023-04-19 14:56     ` Szabolcs Nagy
2023-04-19 15:21       ` Joe Simmons-Talbott
2023-04-19 13:58   ` [PATCH v5 3/3] nptl: Use direct syscall numbers in setxid 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).