lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8390ac81-7774-4e67-9739-c2b98813d6da@csgroup.eu>
Date: Mon, 2 Sep 2024 15:19:56 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "Jason A. Donenfeld" <Jason@...c4.com>,
 Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>
Cc: Will Deacon <will@...nel.org>, Theodore Ts'o <tytso@....edu>,
 linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-arch@...r.kernel.org,
 Catalin Marinas <catalin.marinas@....com>,
 Thomas Gleixner <tglx@...utronix.de>, Eric Biggers <ebiggers@...nel.org>,
 ardb@...nel.org
Subject: Re: [PATCH v2] aarch64: vdso: Wire up getrandom() vDSO implementation



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

Christophe

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ