[PATCH v2] aarch64: vdso: Wire up getrandom() vDSO implementation

Jason A. Donenfeld Jason at zx2c4.com
Mon Sep 2 06:25:34 PDT 2024


On Mon, Sep 02, 2024 at 03:19:56PM +0200, Christophe Leroy wrote:
> 
> 
> Le 02/09/2024 à 15:11, Jason A. Donenfeld a écrit :
> > Hey Christophe (for header logic) & Will (for arm64 stuff),
> > 
> > On Fri, Aug 30, 2024 at 09:28:29AM -0300, Adhemerval Zanella Netto wrote:
> >>>> diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
> >>>> index 938ca539aaa6..7c9711248d9b 100644
> >>>> --- a/lib/vdso/getrandom.c
> >>>> +++ b/lib/vdso/getrandom.c
> >>>> @@ -5,6 +5,7 @@
> >>>>   
> >>>>   #include <linux/array_size.h>
> >>>>   #include <linux/minmax.h>
> >>>> +#include <linux/mm.h>
> >>>>   #include <vdso/datapage.h>
> >>>>   #include <vdso/getrandom.h>
> >>>>   #include <vdso/unaligned.h>
> >>>
> >>> Looks like this should be a separate change?
> >>
> >>
> >> It is required so arm64 can use  c-getrandom-y, otherwise vgetrandom.o build
> >> fails:
> >>
> >> CC      arch/arm64/kernel/vdso/vgetrandom.o
> >> In file included from ./include/uapi/linux/mman.h:5,
> >>                   from /mnt/projects/linux/linux-git/lib/vdso/getrandom.c:13,
> >>                   from <command-line>:
> >> ./arch/arm64/include/asm/mman.h: In function ‘arch_calc_vm_prot_bits’:
> >> ./arch/arm64/include/asm/mman.h:14:13: error: implicit declaration of function ‘system_supports_bti’ [-Werror=implicit-function-declaration]
> >>     14 |         if (system_supports_bti() && (prot & PROT_BTI))
> >>        |             ^~~~~~~~~~~~~~~~~~~
> >> ./arch/arm64/include/asm/mman.h:15:24: error: ‘VM_ARM64_BTI’ undeclared (first use in this function); did you mean ‘ARM64_BTI’?
> >>     15 |                 ret |= VM_ARM64_BTI;
> >>        |                        ^~~~~~~~~~~~
> >>        |                        ARM64_BTI
> >> ./arch/arm64/include/asm/mman.h:15:24: note: each undeclared identifier is reported only once for each function it appears in
> >> ./arch/arm64/include/asm/mman.h:17:13: error: implicit declaration of function ‘system_supports_mte’ [-Werror=implicit-function-declaration]
> >>     17 |         if (system_supports_mte() && (prot & PROT_MTE))
> >>        |             ^~~~~~~~~~~~~~~~~~~
> >> ./arch/arm64/include/asm/mman.h:18:24: error: ‘VM_MTE’ undeclared (first use in this function)
> >>     18 |                 ret |= VM_MTE;
> >>        |                        ^~~~~~
> >> ./arch/arm64/include/asm/mman.h: In function ‘arch_calc_vm_flag_bits’:
> >> ./arch/arm64/include/asm/mman.h:32:24: error: ‘VM_MTE_ALLOWED’ undeclared (first use in this function)
> >>     32 |                 return VM_MTE_ALLOWED;
> >>        |                        ^~~~~~~~~~~~~~
> >> ./arch/arm64/include/asm/mman.h: In function ‘arch_validate_flags’:
> >> ./arch/arm64/include/asm/mman.h:59:29: error: ‘VM_MTE’ undeclared (first use in this function)
> >>     59 |         return !(vm_flags & VM_MTE) || (vm_flags & VM_MTE_ALLOWED);
> >>        |                             ^~~~~~
> >> ./arch/arm64/include/asm/mman.h:59:52: error: ‘VM_MTE_ALLOWED’ undeclared (first use in this function)
> >>     59 |         return !(vm_flags & VM_MTE) || (vm_flags & VM_MTE_ALLOWED);
> >>        |                                                    ^~~~~~~~~~~~~~
> >> arch/arm64/kernel/vdso/vgetrandom.c: In function ‘__kernel_getrandom’:
> >> arch/arm64/kernel/vdso/vgetrandom.c:18:25: error: ‘ENOSYS’ undeclared (first use in this function); did you mean ‘ENOSPC’?
> >>     18 |                 return -ENOSYS;
> >>        |                         ^~~~~~
> >>        |                         ENOSPC
> >> cc1: some warnings being treated as errors
> >>
> >> I can move to a different patch, but this is really tied to this patch.
> > 
> > Adhemerval kept this change in this patch for v3, which, if it's
> > necessary, is fine with me. But I was looking to see if there was
> > another way of doing it, because including linux/mm.h inside of vdso
> > code is kind of contrary to your project with e379299fe0b3 ("random:
> > vDSO: minimize and simplify header includes").
> > 
> > getrandom.c includes uapi/linux/mman.h for the mmap constants. That
> > seems fine; it's userspace code after all. But then uapi/linux/mman.h
> > has this:
> > 
> >     #include <asm/mman.h>
> >     #include <asm-generic/hugetlb_encode.h>
> >     #include <linux/types.h>
> > 
> > The asm-generic/ one resolves to uapi/asm-generic. But the asm/ one
> > resolves to arch code, which is where we then get in trouble on ARM,
> > where arch/arm64/include/asm/mman.h has all sorts of kernel code in it.
> > 
> > Maybe, instead, it should resolve to arch/arm64/include/uapi/asm/mman.h,
> > which is the header that userspace actually uses in normal user code?
> > 
> > Is this a makefile problem? What's going on here? Seems like this is
> > something worth sorting out. Or I can take Adhemerval's v3 as-is and
> > we'll grit our teeth and work it out later, as you prefer. But I thought
> > I should mention it.
> 
> That's a tricky problem, I also have it on powerpc, see patch 5, I 
> solved it that way:
> 
> In the Makefile:
> -ccflags-y := -fno-common -fno-builtin
> +ccflags-y := -fno-common -fno-builtin -DBUILD_VDSO
> 
> In arch/powerpc/include/asm/mman.h:
> 
> diff --git a/arch/powerpc/include/asm/mman.h 
> b/arch/powerpc/include/asm/mman.h
> index 17a77d47ed6d..42a51a993d94 100644
> --- a/arch/powerpc/include/asm/mman.h
> +++ b/arch/powerpc/include/asm/mman.h
> @@ -6,7 +6,7 @@
> 
>   #include <uapi/asm/mman.h>
> 
> -#ifdef CONFIG_PPC64
> +#if defined(CONFIG_PPC64) && !defined(BUILD_VDSO)
> 
>   #include <asm/cputable.h>
>   #include <linux/mm.h>
> 
> So that the only thing that remains in arch/powerpc/include/asm/mman.h 
> when building a VDSO is #include <uapi/asm/mman.h>
> 
> I got the idea from ARM64, they use something similar in their 
> arch/arm64/include/asm/rwonce.h

That seems reasonable enough. Adhemerval - do you want to incorporate
this solution for your v+1? And Will, is it okay to keep that as one
patch, as Christophe has done, rather than splitting it, so the whole
change is hermetic?

Jason



More information about the linux-arm-kernel mailing list