Re: [PATCH v2 1/8] x86: vdso: Introduce asm/vdso/mman.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Le 25/09/2024 à 23:23, Arnd Bergmann a écrit :
On Wed, Sep 25, 2024, at 06:51, Christophe Leroy wrote:
Le 23/09/2024 à 16:19, Vincenzo Frascino a écrit :
@@ -0,0 +1,15 @@
+
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_VDSO_MMAN_H
+#define __ASM_VDSO_MMAN_H
+
+#ifndef __ASSEMBLY__
+
+#include <uapi/linux/mman.h>
+
+#define VDSO_MMAP_PROT	PROT_READ | PROT_WRITE
+#define VDSO_MMAP_FLAGS	MAP_DROPPABLE | MAP_ANONYMOUS

I still can't see the point with that change.

Today 4 architectures implement getrandom and none of them require that
indirection. Please leave prot and flags as they are in the code.

Then this file is totally pointless, VDSO code can include
uapi/linux/mman.h directly.

VDSO is userland code, it should be safe to include any UAPI file there.

I think we are hitting an unfortunate corner case in the build
system here, based on the way we handle the uapi/ file namespace
in the kernel:

include/uapi/linux/mman.h includes three headers: asm/mman.h,
asm-generic/hugetlb_encode.h and linux/types.h. Two of these
exist in both include/uapi/ and include/, so while building
kernel code we end up picking up the non-uapi version which
on some architectures includes many other headers.

Right, and that's the reason why arm64 and powerpc guarded the content of asm/mman.h which an #ifndef BUILD_VDSO.

Note that arm64 also has a similar workaround in asm/rwonce.h, brought by commit e35123d83ee3 ("arm64: lto: Strengthen READ_ONCE() to acquire when CONFIG_LTO=y") without explaination on why VDSO builds are excluded.


I agree that moving the contents out of uapi/ into vdso/ namespace
is not a solution here because that removes the contents from
the installed user headers, but we still need to do something
to solve the issue.

Should header inclusion be reworked so that only UAPI and VDSO pathes are looked for when including headers in VDSO builds ?


The easiest workaround I see for this particular file is to
move the contents of arch/{arm,arm64,parisc,powerpc,sparc,x86}/\
include/asm/mman.h into a different file to ensure that the
only existing file is the uapi/ one. Unfortunately this does
not help to avoid it regressing again in the future.

Could we add a check in checkpatch.pl to ensure UAPI headers do not include headers that exist both in UAPI and non-UAPI space in the future ?

Christophe




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux