2023-04-04 12:35:23

by Josh Triplett

[permalink] [raw]
Subject: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace

If a library wants to get information from auxv (for instance,
AT_HWCAP/AT_HWCAP2), it has a few options, none of them perfectly
reliable or ideal:

- Be main or the pre-main startup code, and grub through the stack above
main. Doesn't work for a library.
- Call libc getauxval. Not ideal for libraries that are trying to be
libc-independent and/or don't otherwise require anything from other
libraries.
- Open and read /proc/self/auxv. Doesn't work for libraries that may run
in arbitrarily constrained environments that may not have /proc
mounted (e.g. libraries that might be used by an init program or a
container setup tool).
- Assume you're on the main thread and still on the original stack, and
try to walk the stack upwards, hoping to find auxv. Extremely bad
idea.
- Ask the caller to pass auxv in for you. Not ideal for a user-friendly
library, and then your caller may have the same problem.

Add a prctl that copies current->mm->saved_auxv to a userspace buffer.

Signed-off-by: Josh Triplett <[email protected]>
---

v2:
- Fix W=1 warning about min type mismatch

I've built, booted, and tested this.

man-pages snippet:

.\" prctl PR_GET_AUXV
.TP
.BR PR_GET_AUXV " (since Linux 6.x)"
Get the auxilliary vector (auxv) into the buffer pointed to by
.IR "(void\~*) arg2" ,
whose length is given by \fIarg3\fP.
If the buffer is not long enough for the full auxilliary vector,
the copy will be truncated.
Return (as the function result)
the full length of the auxilliary vector.
\fIarg4\fP and \fIarg5\fP must be 0.

Will send a patch for man-pages once merged.

include/uapi/linux/prctl.h | 2 ++
kernel/sys.c | 15 +++++++++++++++
2 files changed, 17 insertions(+)

diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 1312a137f7fb..b2b24eaf2427 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -290,4 +290,6 @@ struct prctl_mm_map {
#define PR_SET_VMA 0x53564d41
# define PR_SET_VMA_ANON_NAME 0

+#define PR_GET_AUXV 0x41555856
+
#endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 495cd87d9bf4..43f922170706 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2377,6 +2377,16 @@ static inline int prctl_get_mdwe(unsigned long arg2, unsigned long arg3,
PR_MDWE_REFUSE_EXEC_GAIN : 0;
}

+static int prctl_get_auxv(void __user *addr, unsigned long len)
+{
+ struct mm_struct *mm = current->mm;
+ unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);
+
+ if (size && copy_to_user(addr, mm->saved_auxv, size))
+ return -EFAULT;
+ return sizeof(mm->saved_auxv);
+}
+
SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
unsigned long, arg4, unsigned long, arg5)
{
@@ -2661,6 +2671,11 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
case PR_SET_VMA:
error = prctl_set_vma(arg2, arg3, arg4, arg5);
break;
+ case PR_GET_AUXV:
+ if (arg4 || arg5)
+ return -EINVAL;
+ error = prctl_get_auxv((void __user *)arg2, arg3);
+ break;
default:
error = -EINVAL;
break;
--
2.40.0


2023-04-04 19:55:22

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace

On Tue, 4 Apr 2023 21:31:48 +0900 Josh Triplett <[email protected]> wrote:

> If a library wants to get information from auxv (for instance,
> AT_HWCAP/AT_HWCAP2), it has a few options, none of them perfectly
> reliable or ideal:
>
> - Be main or the pre-main startup code, and grub through the stack above
> main. Doesn't work for a library.
> - Call libc getauxval. Not ideal for libraries that are trying to be
> libc-independent and/or don't otherwise require anything from other
> libraries.
> - Open and read /proc/self/auxv. Doesn't work for libraries that may run
> in arbitrarily constrained environments that may not have /proc
> mounted (e.g. libraries that might be used by an init program or a
> container setup tool).
> - Assume you're on the main thread and still on the original stack, and
> try to walk the stack upwards, hoping to find auxv. Extremely bad
> idea.
> - Ask the caller to pass auxv in for you. Not ideal for a user-friendly
> library, and then your caller may have the same problem.

How does glibc's getauxval() do its thing? Why can't glibc-independent
code do the same thing?

> Add a prctl that copies current->mm->saved_auxv to a userspace buffer.
>
> ...
>
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -290,4 +290,6 @@ struct prctl_mm_map {
> #define PR_SET_VMA 0x53564d41
> # define PR_SET_VMA_ANON_NAME 0
>
> +#define PR_GET_AUXV 0x41555856

How was this constant arrived at?

> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2377,6 +2377,16 @@ static inline int prctl_get_mdwe(unsigned long arg2, unsigned long arg3,
> PR_MDWE_REFUSE_EXEC_GAIN : 0;
> }
>
> +static int prctl_get_auxv(void __user *addr, unsigned long len)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);
> +
> + if (size && copy_to_user(addr, mm->saved_auxv, size))
> + return -EFAULT;
> + return sizeof(mm->saved_auxv);
> +}

The type choices are unpleasing. Maybe make `len' a size_t and make
the function return a size_t? That way prctl_get_auxv() will be much
nicer, but the caller less so.

> SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> unsigned long, arg4, unsigned long, arg5)
> {
> @@ -2661,6 +2671,11 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
> case PR_SET_VMA:
> error = prctl_set_vma(arg2, arg3, arg4, arg5);
> break;
> + case PR_GET_AUXV:
> + if (arg4 || arg5)
> + return -EINVAL;
> + error = prctl_get_auxv((void __user *)arg2, arg3);
> + break;
> default:
> error = -EINVAL;
> break;

2023-04-05 00:30:34

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace

On Wed, Apr 05, 2023 at 09:24:36AM +0900, Josh Triplett wrote:
> On Tue, Apr 04, 2023 at 12:43:55PM -0700, Andrew Morton wrote:
> > On Tue, 4 Apr 2023 21:31:48 +0900 Josh Triplett <[email protected]> wrote:
> > > --- a/kernel/sys.c
> > > +++ b/kernel/sys.c
> > > @@ -2377,6 +2377,16 @@ static inline int prctl_get_mdwe(unsigned long arg2, unsigned long arg3,
> > > PR_MDWE_REFUSE_EXEC_GAIN : 0;
> > > }
> > >
> > > +static int prctl_get_auxv(void __user *addr, unsigned long len)
> > > +{
> > > + struct mm_struct *mm = current->mm;
> > > + unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);
> > > +
> > > + if (size && copy_to_user(addr, mm->saved_auxv, size))
> > > + return -EFAULT;
> > > + return sizeof(mm->saved_auxv);
> > > +}
> >
> > The type choices are unpleasing. Maybe make `len' a size_t and make
> > the function return a size_t? That way prctl_get_auxv() will be much
> > nicer, but the caller less so.
>
> It'd have to be an ssize_t return to support returning -EFAULT. Also,
> sadly, size_t would still look just as bad, because
> `sizeof(mm->saved_auxv)` doesn't have type size_t (at least according to
> the error from the type-safe min macro). So this would still need a cast
> or a `min_t`.
>
> But I'm happy to change the argument to size_t and the return value to
> ssize_t, if you'd prefer. Will send v3 with that changed.

That said, *all* the other helper functions here seem to return int...

2023-04-05 00:32:23

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace

On Tue, Apr 04, 2023 at 12:43:55PM -0700, Andrew Morton wrote:
> On Tue, 4 Apr 2023 21:31:48 +0900 Josh Triplett <[email protected]> wrote:
>
> > If a library wants to get information from auxv (for instance,
> > AT_HWCAP/AT_HWCAP2), it has a few options, none of them perfectly
> > reliable or ideal:
> >
> > - Be main or the pre-main startup code, and grub through the stack above
> > main. Doesn't work for a library.
> > - Call libc getauxval. Not ideal for libraries that are trying to be
> > libc-independent and/or don't otherwise require anything from other
> > libraries.
> > - Open and read /proc/self/auxv. Doesn't work for libraries that may run
> > in arbitrarily constrained environments that may not have /proc
> > mounted (e.g. libraries that might be used by an init program or a
> > container setup tool).
> > - Assume you're on the main thread and still on the original stack, and
> > try to walk the stack upwards, hoping to find auxv. Extremely bad
> > idea.
> > - Ask the caller to pass auxv in for you. Not ideal for a user-friendly
> > library, and then your caller may have the same problem.
>
> How does glibc's getauxval() do its thing? Why can't glibc-independent
> code do the same thing?

glibc owns the pre-main startup code in programs linked to glibc, so it
can record auxv for later reference in getauxval. That isn't an option
for something that *doesn't* own the pre-main startup code.

> > --- a/include/uapi/linux/prctl.h
> > +++ b/include/uapi/linux/prctl.h
> > @@ -290,4 +290,6 @@ struct prctl_mm_map {
> > #define PR_SET_VMA 0x53564d41
> > # define PR_SET_VMA_ANON_NAME 0
> >
> > +#define PR_GET_AUXV 0x41555856
>
> How was this constant arrived at?

It's 'A' 'U' 'X' 'V', inspired by PR_SET_VMA above which is 'S' 'V' 'M' 'A'.

> > --- a/kernel/sys.c
> > +++ b/kernel/sys.c
> > @@ -2377,6 +2377,16 @@ static inline int prctl_get_mdwe(unsigned long arg2, unsigned long arg3,
> > PR_MDWE_REFUSE_EXEC_GAIN : 0;
> > }
> >
> > +static int prctl_get_auxv(void __user *addr, unsigned long len)
> > +{
> > + struct mm_struct *mm = current->mm;
> > + unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);
> > +
> > + if (size && copy_to_user(addr, mm->saved_auxv, size))
> > + return -EFAULT;
> > + return sizeof(mm->saved_auxv);
> > +}
>
> The type choices are unpleasing. Maybe make `len' a size_t and make
> the function return a size_t? That way prctl_get_auxv() will be much
> nicer, but the caller less so.

It'd have to be an ssize_t return to support returning -EFAULT. Also,
sadly, size_t would still look just as bad, because
`sizeof(mm->saved_auxv)` doesn't have type size_t (at least according to
the error from the type-safe min macro). So this would still need a cast
or a `min_t`.

But I'm happy to change the argument to size_t and the return value to
ssize_t, if you'd prefer. Will send v3 with that changed.

- Josh Triplett

2023-04-05 10:50:45

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v2] prctl: Add PR_GET_AUXV to copy auxv to userspace

From: Josh Triplett
> Sent: 04 April 2023 13:32
>
> If a library wants to get information from auxv (for instance,
> AT_HWCAP/AT_HWCAP2), it has a few options, none of them perfectly
> reliable or ideal:
>
> - Be main or the pre-main startup code, and grub through the stack above
> main. Doesn't work for a library.
> - Call libc getauxval. Not ideal for libraries that are trying to be
> libc-independent and/or don't otherwise require anything from other
> libraries.
> - Open and read /proc/self/auxv. Doesn't work for libraries that may run
> in arbitrarily constrained environments that may not have /proc
> mounted (e.g. libraries that might be used by an init program or a
> container setup tool).
> - Assume you're on the main thread and still on the original stack, and
> try to walk the stack upwards, hoping to find auxv. Extremely bad
> idea.
> - Ask the caller to pass auxv in for you. Not ideal for a user-friendly
> library, and then your caller may have the same problem.
>
> Add a prctl that copies current->mm->saved_auxv to a userspace buffer.
...
> +static int prctl_get_auxv(void __user *addr, unsigned long len)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long size = min_t(unsigned long, sizeof(mm->saved_auxv), len);

Don't use min_t() fix the types.
min_t() is a horrid abomination that is massively overused.

It would be better to have a min_unchecked() that just skips the
type test.

Or accept my patches that allows allow min/max against
compile-time constants between 0 and MAX_INT.
After all, the only reason for the type check is to try
to avoid negative values becoming large positive ones
due to integer promotions.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)