Le 27/08/2024 à 11:59, Arnd Bergmann a écrit :
On Tue, Aug 27, 2024, at 10:40, Jason A. Donenfeld wrote:
I don't love this, but it might be the lesser of evils, so sure, let's
do it.
I think I'll combine these header fixups so that the whole operation is
a bit more clear. The commit is still pretty small. Something like
below:
From 0d9a3d68cd6222395a605abd0ac625c41d4cabfa Mon Sep 17 00:00:00 2001
From: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
Date: Tue, 27 Aug 2024 09:31:47 +0200
Subject: [PATCH] random: vDSO: clean header inclusion in getrandom
Depending on the architecture, building a 32-bit vDSO on a 64-bit kernel
is problematic when some system headers are included.
Minimise the amount of headers by moving needed items, such as
__{get,put}_unaligned_t, into dedicated common headers and in general
use more specific headers, similar to what was done in commit
8165b57bca21 ("linux/const.h: Extract common header for vDSO") and
commit 8c59ab839f52 ("lib/vdso: Enable common headers").
On some architectures this results in missing PAGE_SIZE, as was
described by commit 8b3843ae3634 ("vdso/datapage: Quick fix - use
asm/page-def.h for ARM64"), so define this if necessary, in the same way
as done prior by commit cffaefd15a8f ("vdso: Use CONFIG_PAGE_SHIFT in
vdso/datapage.h").
Removing linux/time64.h leads to missing 'struct timespec64' in
x86's asm/pvclock.h. Add a forward declaration of that struct in
that file.
Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
This is clearly better, but there are still a couple of inaccuracies
that may end up biting us again later. Not sure whether it's worth
trying to fix it all at once or if we want to address them when that
happens:
#include <linux/array_size.h>
-#include <linux/cache.h>
-#include <linux/kernel.h>
-#include <linux/time64.h>
+#include <linux/minmax.h>
These are still two headers outside of the vdso/ namespace. For arm64
we had concluded that this is never safe, and any vdso header should
only include other vdso headers so we never pull in anything that
e.g. depends on memory management headers that are in turn broken
for the compat vdso.
The array_size.h header is really small, so that one could
probably just be moved into the vdso/ namespace. The minmax.h
header is already rather complex, so it may be better to just
open-code the usage of MIN/MAX where needed?
It is used at two places only so yes can to that.
Same for ARRAY_SIZE(->reserved) by the way, easy to do opencode, we also
have it only once
#include <vdso/datapage.h>
#include <vdso/getrandom.h>
+#include <vdso/unaligned.h>
#include <asm/vdso/getrandom.h>
-#include <asm/vdso/vsyscall.h>
-#include <asm/unaligned.h>
#include <uapi/linux/mman.h>
+#include <uapi/linux/random.h>
+
+#undef PAGE_SIZE
+#undef PAGE_MASK
+#define PAGE_SIZE (1UL << CONFIG_PAGE_SHIFT)
+#define PAGE_MASK (~(PAGE_SIZE - 1))
Since these are now the same across all architectures, maybe we
can just have the PAGE_SIZE definitions a vdso header instead
and include that from asm/page.h.
I gave it a quick look yesterday, there are still some subtleties
between architectures.
For instance, most architectures use 1UL for the shift but powerpc use 1
and has the following comment:
/*
* Subtle: (1 << PAGE_SHIFT) is an int, not an unsigned long. So if we
* assign PAGE_MASK to a larger type it gets extended the way we want
* (i.e. with 1s in the high bits)
*/
So we'll have to look at all this carefully when we want something
common, or am I missing something ?
Including uapi/linux/mman.h may still be problematic on
some architectures if they change it in a way that is
incompatible with compat vdso, but at least that can't
accidentally rely on CONFIG_64BIT or something else that
would be wrong there.
Yes that one is tricky. Because uapi/linux/mman.h includes asm/mman.h
with the intention to include uapi/asm/mman.h but when built from the
kernel in reality you get arch/powerpc/include/asm/mman.h and I had to
add some ifdefery to kick-out kernel oddities it contains that pull
additional kernel headers.
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>
Christophe
