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

Christophe Leroy
2024-09-04T16:56:38Z

Le 03/09/2024 à 17:14, Vincenzo Frascino a écrit :
> The VDSO implementation includes headers from outside of the
> vdso/ namespace.
>
> Introduce asm/vdso/mman.h to make sure that the generic library
> uses only the allowed namespace.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> ---
> arch/x86/include/asm/vdso/mman.h | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> create mode 100644 arch/x86/include/asm/vdso/mman.h
>
> diff --git a/arch/x86/include/asm/vdso/mman.h b/arch/x86/include/asm/vdso/mman.h
> new file mode 100644
> index 000000000000..4c936c9d11ab
> --- /dev/null
> +++ b/arch/x86/include/asm/vdso/mman.h
> @@ -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 benefit of duplicating those two defines in every
arch.

I understand your point that some arch might in the future want to use
different flags, but unless we already have one such architecture in
mind we shouldn't make things more complicated than needed.

In case such an architecture is already identified it should be said in
the commit message

> +
> +#endif /* !__ASSEMBLY__ */
> +
> +#endif /* __ASM_VDSO_MMAN_H */