V3 [PATCH] Add a C wrapper for prctl [BZ #25896]
Commit Message
On Thu, Apr 30, 2020 at 7:04 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>
> On Apr 30 2020, H.J. Lu wrote:
>
> > On Thu, Apr 30, 2020 at 6:51 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> >>
> >> On Apr 30 2020, Florian Weimer wrote:
> >>
> >> > * Andreas Schwab:
> >> >
> >> >> On Apr 30 2020, H.J. Lu via Libc-alpha wrote:
> >> >>
> >> >>> diff --git a/include/sys/prctl.h b/include/sys/prctl.h
> >> >>> index 0920ed642b..1a74d83879 100644
> >> >>> --- a/include/sys/prctl.h
> >> >>> +++ b/include/sys/prctl.h
> >> >>> @@ -4,6 +4,8 @@
> >> >>> # ifndef _ISOMAC
> >> >>>
> >> >>> extern int __prctl (int __option, ...);
> >> >>> +libc_hidden_proto (__prctl)
> >> >>> +libc_hidden_proto (prctl)
> >> >>
> >> >> Why do you need the hidden alias for prctl? All references inside
> >> >> libc.so should use __prctl.
> >> >
> >> > Right. I want to clean this up in a separate patch.
> >>
> >> ??? Why adding it in the first place?
> >>
> >> Andreas.
> >>
> >
> > It was there before my patch:
>
> That doesn't matter, the syscall template adds it blindly to all names.
>
> Andreas.
>
Here is the updated patch without
libc_hidden_proto (prctl)
OK for master?
Thanks.
Comments
* 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
@@ -4,6 +4,7 @@
# ifndef _ISOMAC
extern int __prctl (int __option, ...);
+libc_hidden_proto (__prctl)
# endif /* !_ISOMAC */
#endif
@@ -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
new file mode 100644
@@ -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)
@@ -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
new file mode 100644
@@ -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)
new file mode 100644
@@ -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