[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZCy/vgprgeVUwCGv@localhost>
Date: Wed, 5 Apr 2023 09:24:30 +0900
From: Josh Triplett <josh@...htriplett.org>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org
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 <josh@...htriplett.org> 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
Powered by blists - more mailing lists