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

["Jason A. Donenfeld" <Jason@zx2c4.com>]

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.

Thoughts?

Jason