Message ID | CAMe9rOok1_-PuzwExk+XZJ_KyE01keNstYAGS9GR9gn6GFHxvg@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | V3 [PATCH] Add a C wrapper for prctl [BZ #25896] | expand |
* H. J. Lu via Libc-alpha: > From ee8672af3ef5f3db438c0abb39b2673944181292 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Wed, 29 Apr 2020 07:38:49 -0700 > Subject: [PATCH] Add a C wrapper for prctl [BZ #25896] > > Add a C wrapper to pass arguments in The commit message is only correct if you drop the .S files. > /* Control process execution. */ > extern int prctl (int __option, ...) __THROW; > > to prctl syscall: > > extern int prctl (int, unsigned long int, unsigned long int, > unsigned long int, unsigned long int); > > On Linux/x86, since the prctl syscall interface: > > extern int prctl (int, unsigned long int, unsigned long int, > unsigned long int, unsigned long int); > > and the glibc prctl interface: > > extern int prctl (int option, ...); > > pass the arguments identically, the assembly verion: > > PSEUDO (__prctl, prctl, 5) > ret > PSEUDO_END (__prctl) > > is used. Doesn't x86 include x32? Apart from these nits, the patch looks good. Please settle the matter of the .S files with Adhemerval. 8-)
On Thu, Apr 30, 2020 at 9:53 AM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu via Libc-alpha: > > > From ee8672af3ef5f3db438c0abb39b2673944181292 Mon Sep 17 00:00:00 2001 > > From: "H.J. Lu" <hjl.tools@gmail.com> > > Date: Wed, 29 Apr 2020 07:38:49 -0700 > > Subject: [PATCH] Add a C wrapper for prctl [BZ #25896] > > > > Add a C wrapper to pass arguments in > > The commit message is only correct if you drop the .S files. > > > /* Control process execution. */ > > extern int prctl (int __option, ...) __THROW; > > > > to prctl syscall: > > > > extern int prctl (int, unsigned long int, unsigned long int, > > unsigned long int, unsigned long int); > > > > On Linux/x86, since the prctl syscall interface: > > > > extern int prctl (int, unsigned long int, unsigned long int, > > unsigned long int, unsigned long int); > > > > and the glibc prctl interface: > > > > extern int prctl (int option, ...); > > > > pass the arguments identically, the assembly verion: > > > > PSEUDO (__prctl, prctl, 5) > > ret > > PSEUDO_END (__prctl) > > > > is used. > > Doesn't x86 include x32? Yes. > Apart from these nits, the patch looks good. Please settle the matter > of the .S files with Adhemerval. 8-) Since the C version is so much worse than the assembler version, I prefer to include the assembler wrapper. But I won't insist. Adhemerval, please comment. Thanks.
* H. J. Lu: > Here is the updated patch without > > libc_hidden_proto (prctl) > > OK for master? Looks good to me.
* H. J. Lu via Libc-alpha: > From ee8672af3ef5f3db438c0abb39b2673944181292 Mon Sep 17 00:00:00 2001 > From: "H.J. Lu" <hjl.tools@gmail.com> > Date: Wed, 29 Apr 2020 07:38:49 -0700 > Subject: [PATCH] Add a C wrapper for prctl [BZ #25896] > > Add a C wrapper to pass arguments in > > /* Control process execution. */ > extern int prctl (int __option, ...) __THROW; > > to prctl syscall: > > extern int prctl (int, unsigned long int, unsigned long int, > unsigned long int, unsigned long int); > > On Linux/x86, since the prctl syscall interface: > > extern int prctl (int, unsigned long int, unsigned long int, > unsigned long int, unsigned long int); > > and the glibc prctl interface: > > extern int prctl (int option, ...); > > pass the arguments identically, the assembly verion: > > PSEUDO (__prctl, prctl, 5) > ret > PSEUDO_END (__prctl) > > is used. This broke ABI on powerpc64le-linux-gnu because the calling convention is not identical. The manual page specifies the second prototype. I filed a GCC RFE so that we can deal with this in a more elegant manner: rs6000: Option not to use parameter save area in variadic function implementations <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107606> I don't know whether we should restrict the C prctl wrapper to x86 x32, or if we should add an assembler wrapper on powerpc64le-linux-gnu. Thanks, Florian
On 10/11/22 05:33, Florian Weimer via Libc-alpha wrote: > * H. J. Lu via Libc-alpha: > >> From ee8672af3ef5f3db438c0abb39b2673944181292 Mon Sep 17 00:00:00 2001 >> From: "H.J. Lu" <hjl.tools@gmail.com> >> Date: Wed, 29 Apr 2020 07:38:49 -0700 >> Subject: [PATCH] Add a C wrapper for prctl [BZ #25896] >> >> Add a C wrapper to pass arguments in >> >> /* Control process execution. */ >> extern int prctl (int __option, ...) __THROW; >> >> to prctl syscall: >> >> extern int prctl (int, unsigned long int, unsigned long int, >> unsigned long int, unsigned long int); >> >> On Linux/x86, since the prctl syscall interface: >> >> extern int prctl (int, unsigned long int, unsigned long int, >> unsigned long int, unsigned long int); >> >> and the glibc prctl interface: >> >> extern int prctl (int option, ...); >> >> pass the arguments identically, the assembly verion: >> >> PSEUDO (__prctl, prctl, 5) >> ret >> PSEUDO_END (__prctl) >> >> is used. > > This broke ABI on powerpc64le-linux-gnu because the calling convention > is not identical. The manual page specifies the second prototype. > > I filed a GCC RFE so that we can deal with this in a more elegant > manner: > > rs6000: Option not to use parameter save area in variadic function > implementations > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107606> > > I don't know whether we should restrict the C prctl wrapper to x86 x32, > or if we should add an assembler wrapper on powerpc64le-linux-gnu. > > Thanks, > Florian > The previous syscalls.list entry marked the function as 5 argument one instead of variadic, so I think it would be better to add a way each ABI to use this instead of the variadic one. Something like: #if PRCTL_VARIADIC_OK int __prctl (int option, ...) { va_list arg; va_start (arg, option); unsigned long int arg2 = va_arg (arg, unsigned long int); unsigned long int arg3 = va_arg (arg, unsigned long int); unsigned long int arg4 = va_arg (arg, unsigned long int); unsigned long int arg5 = va_arg (arg, unsigned long int); va_end (arg); return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); } #else int __prctl (int option, int arg1, int arg2, int arg3, int arg4, int arg5) { return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); } #endif Or if powerpc64le is the only affected add a specific C implementation for it. I think we should really move away from assembler wrappers.
* Adhemerval Zanella Netto: > The previous syscalls.list entry marked the function as 5 argument one > instead of variadic, so I think it would be better to add a way each > ABI to use this instead of the variadic one. Something like: > > #if PRCTL_VARIADIC_OK > int > __prctl (int option, ...) > { > va_list arg; > va_start (arg, option); > unsigned long int arg2 = va_arg (arg, unsigned long int); > unsigned long int arg3 = va_arg (arg, unsigned long int); > unsigned long int arg4 = va_arg (arg, unsigned long int); > unsigned long int arg5 = va_arg (arg, unsigned long int); > va_end (arg); > return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); > } > #else > int > __prctl (int option, int arg1, int arg2, int arg3, int arg4, int arg5) > { > return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); > } > #endif (The arguments need to be unsigned long int.) This is not easy to do in C because the GCC aliasing machinery performs type checking (as it should). We can probably use #define prctl prctl_XXX #define __prctl __prctl_XXX before including <sys/prctl.h>, but that's quite ugly. Do you still want me to proceed with the C version? Thanks, Florian
On 10/11/22 09:03, Florian Weimer wrote: > * Adhemerval Zanella Netto: > >> The previous syscalls.list entry marked the function as 5 argument one >> instead of variadic, so I think it would be better to add a way each >> ABI to use this instead of the variadic one. Something like: >> >> #if PRCTL_VARIADIC_OK >> int >> __prctl (int option, ...) >> { >> va_list arg; >> va_start (arg, option); >> unsigned long int arg2 = va_arg (arg, unsigned long int); >> unsigned long int arg3 = va_arg (arg, unsigned long int); >> unsigned long int arg4 = va_arg (arg, unsigned long int); >> unsigned long int arg5 = va_arg (arg, unsigned long int); >> va_end (arg); >> return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); >> } >> #else >> int >> __prctl (int option, int arg1, int arg2, int arg3, int arg4, int arg5) >> { >> return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); >> } >> #endif > > (The arguments need to be unsigned long int.) > > This is not easy to do in C because the GCC aliasing machinery performs > type checking (as it should). > > We can probably use > > #define prctl prctl_XXX > #define __prctl __prctl_XXX Yes, I did not include it but we already do it for non-LFS to LFS alias. > > before including <sys/prctl.h>, but that's quite ugly. Do you still > want me to proceed with the C version? I would, although it is not the simplest solution.
* Adhemerval Zanella Netto: > On 10/11/22 09:03, Florian Weimer wrote: >> * Adhemerval Zanella Netto: >> >>> The previous syscalls.list entry marked the function as 5 argument one >>> instead of variadic, so I think it would be better to add a way each >>> ABI to use this instead of the variadic one. Something like: >>> >>> #if PRCTL_VARIADIC_OK >>> int >>> __prctl (int option, ...) >>> { >>> va_list arg; >>> va_start (arg, option); >>> unsigned long int arg2 = va_arg (arg, unsigned long int); >>> unsigned long int arg3 = va_arg (arg, unsigned long int); >>> unsigned long int arg4 = va_arg (arg, unsigned long int); >>> unsigned long int arg5 = va_arg (arg, unsigned long int); >>> va_end (arg); >>> return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); >>> } >>> #else >>> int >>> __prctl (int option, int arg1, int arg2, int arg3, int arg4, int arg5) >>> { >>> return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); >>> } >>> #endif >> >> (The arguments need to be unsigned long int.) >> >> This is not easy to do in C because the GCC aliasing machinery performs >> type checking (as it should). >> >> We can probably use >> >> #define prctl prctl_XXX >> #define __prctl __prctl_XXX > > Yes, I did not include it but we already do it for non-LFS to LFS alias. > >> >> before including <sys/prctl.h>, but that's quite ugly. Do you still >> want me to proceed with the C version? > > I would, although it is not the simplest solution. It's not too bad. I'm going to make the internal __prctl prototype non-variadic. Thanks, Florian
From ee8672af3ef5f3db438c0abb39b2673944181292 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 29 Apr 2020 07:38:49 -0700 Subject: [PATCH] Add a C wrapper for prctl [BZ #25896] Add a C wrapper to pass arguments in /* Control process execution. */ extern int prctl (int __option, ...) __THROW; to prctl syscall: extern int prctl (int, unsigned long int, unsigned long int, unsigned long int, unsigned long int); On Linux/x86, since the prctl syscall interface: extern int prctl (int, unsigned long int, unsigned long int, unsigned long int, unsigned long int); and the glibc prctl interface: extern int prctl (int option, ...); pass the arguments identically, the assembly verion: PSEUDO (__prctl, prctl, 5) ret PSEUDO_END (__prctl) is used. --- include/sys/prctl.h | 1 + sysdeps/unix/sysv/linux/Makefile | 2 +- sysdeps/unix/sysv/linux/prctl.c | 42 ++++++++++++++++++++++ sysdeps/unix/sysv/linux/syscalls.list | 1 - sysdeps/unix/sysv/linux/x86/prctl.S | 37 +++++++++++++++++++ sysdeps/unix/sysv/linux/x86_64/x32/prctl.S | 28 +++++++++++++++ 6 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 sysdeps/unix/sysv/linux/prctl.c create mode 100644 sysdeps/unix/sysv/linux/x86/prctl.S create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/prctl.S diff --git a/include/sys/prctl.h b/include/sys/prctl.h index 0920ed642b..d33f3a290e 100644 --- a/include/sys/prctl.h +++ b/include/sys/prctl.h @@ -4,6 +4,7 @@ # ifndef _ISOMAC extern int __prctl (int __option, ...); +libc_hidden_proto (__prctl) # endif /* !_ISOMAC */ #endif diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index db78eeadd1..0326f92c40 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -59,7 +59,7 @@ sysdep_routines += adjtimex clone umount umount2 readahead sysctl \ eventfd eventfd_read eventfd_write prlimit \ personality epoll_wait tee vmsplice splice \ open_by_handle_at mlock2 pkey_mprotect pkey_set pkey_get \ - timerfd_gettime timerfd_settime \ + timerfd_gettime timerfd_settime prctl \ process_vm_readv process_vm_writev CFLAGS-gethostid.c = -fexceptions diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c new file mode 100644 index 0000000000..d5725f14cf --- /dev/null +++ b/sysdeps/unix/sysv/linux/prctl.c @@ -0,0 +1,42 @@ +/* prctl - Linux specific syscall. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <sysdep.h> +#include <stdarg.h> +#include <sys/prctl.h> + +/* Unconditionally read all potential arguments. This may pass + garbage values to the kernel, but avoids the need for teaching + glibc the argument counts of individual options (including ones + that are added to the kernel in the future). */ + +int +__prctl (int option, ...) +{ + va_list arg; + va_start (arg, option); + unsigned long int arg2 = va_arg (arg, unsigned long int); + unsigned long int arg3 = va_arg (arg, unsigned long int); + unsigned long int arg4 = va_arg (arg, unsigned long int); + unsigned long int arg5 = va_arg (arg, unsigned long int); + va_end (arg); + return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5); +} + +libc_hidden_def (__prctl) +weak_alias (__prctl, prctl) diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list index ced77d49fa..1d8893d1b8 100644 --- a/sysdeps/unix/sysv/linux/syscalls.list +++ b/sysdeps/unix/sysv/linux/syscalls.list @@ -43,7 +43,6 @@ nfsservctl EXTRA nfsservctl i:ipp __compat_nfsservctl nfsservctl@GLIBC_2.0:GLIBC pipe - pipe i:f __pipe pipe pipe2 - pipe2 i:fi __pipe2 pipe2 pivot_root EXTRA pivot_root i:ss pivot_root -prctl EXTRA prctl i:iiiii __prctl prctl query_module EXTRA query_module i:sipip __compat_query_module query_module@GLIBC_2.0:GLIBC_2.23 quotactl EXTRA quotactl i:isip quotactl remap_file_pages - remap_file_pages i:pUiUi __remap_file_pages remap_file_pages diff --git a/sysdeps/unix/sysv/linux/x86/prctl.S b/sysdeps/unix/sysv/linux/x86/prctl.S new file mode 100644 index 0000000000..ce9903ab15 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/prctl.S @@ -0,0 +1,37 @@ +/* prctl - Linux specific syscall. Linux/x86 version. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <sysdep.h> + +/* On Linux/x86, the prctl syscall interface: + + extern int prctl (int, unsigned long int, unsigned long int, + unsigned long int, unsigned long int); + + and the glibc prctl interface: + + extern int prctl (int option, ...); + + pass the arguments identically. */ + +PSEUDO (__prctl, prctl, 5) + ret +PSEUDO_END (__prctl) + +hidden_def (__prctl) +weak_alias (__prctl, prctl) diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S new file mode 100644 index 0000000000..5acf5406a8 --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86_64/x32/prctl.S @@ -0,0 +1,28 @@ +/* prctl - Linux specific syscall. Linux/x32 version. + Copyright (C) 2020 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <sysdep.h> + +/* On Linux/x32, we need to zero-extend from 32 bits to 64 bits for the + last 4 arguments. */ + +#undef DOARGS_5 +#define DOARGS_5 \ + movl %esi, %esi; movl %edx, %edx; movl %ecx, %r10d; movl %r8d, %r8d; + +#include <sysdeps/unix/sysv/linux/x86/prctl.S> -- 2.26.2