[v3,1/2] x86_64: Set the syscall register right before doing the syscall.
Checks
| Context |
Check |
Description |
| dj/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
Commit Message
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(+)
Comments
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?
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
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.
@@ -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) \