2023-09-15 13:01:43

by Vincent Whitchurch

[permalink] [raw]
Subject: [PATCH v2] x86: Fix build of UML with KASAN

Building UML with KASAN fails since commit 69d4c0d32186 ("entry, kasan,
x86: Disallow overriding mem*() functions") with the following errors:

$ tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y
...
ld: mm/kasan/shadow.o: in function `memset':
shadow.c:(.text+0x40): multiple definition of `memset';
arch/x86/lib/memset_64.o:(.noinstr.text+0x0): first defined here
ld: mm/kasan/shadow.o: in function `memmove':
shadow.c:(.text+0x90): multiple definition of `memmove';
arch/x86/lib/memmove_64.o:(.noinstr.text+0x0): first defined here
ld: mm/kasan/shadow.o: in function `memcpy':
shadow.c:(.text+0x110): multiple definition of `memcpy';
arch/x86/lib/memcpy_64.o:(.noinstr.text+0x0): first defined here

UML does not use GENERIC_ENTRY and is still supposed to be allowed to
override the mem*() functions, so use weak aliases in that case.

Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
Signed-off-by: Vincent Whitchurch <[email protected]>
---
Changes in v2:
- Use CONFIG_UML instead of CONFIG_GENERIC_ENTRY.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
arch/x86/lib/memcpy_64.S | 4 ++++
arch/x86/lib/memmove_64.S | 4 ++++
arch/x86/lib/memset_64.S | 4 ++++
3 files changed, 12 insertions(+)

diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 8f95fb267caa..47b004851cf3 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -40,7 +40,11 @@ SYM_TYPED_FUNC_START(__memcpy)
SYM_FUNC_END(__memcpy)
EXPORT_SYMBOL(__memcpy)

+#ifdef CONFIG_UML
+SYM_FUNC_ALIAS_WEAK(memcpy, __memcpy)
+#else
SYM_FUNC_ALIAS(memcpy, __memcpy)
+#endif
EXPORT_SYMBOL(memcpy)

SYM_FUNC_START_LOCAL(memcpy_orig)
diff --git a/arch/x86/lib/memmove_64.S b/arch/x86/lib/memmove_64.S
index 0559b206fb11..e3a76d38c278 100644
--- a/arch/x86/lib/memmove_64.S
+++ b/arch/x86/lib/memmove_64.S
@@ -212,5 +212,9 @@ SYM_FUNC_START(__memmove)
SYM_FUNC_END(__memmove)
EXPORT_SYMBOL(__memmove)

+#ifdef CONFIG_UML
+SYM_FUNC_ALIAS_WEAK(memmove, __memmove)
+#else
SYM_FUNC_ALIAS(memmove, __memmove)
+#endif
EXPORT_SYMBOL(memmove)
diff --git a/arch/x86/lib/memset_64.S b/arch/x86/lib/memset_64.S
index 7c59a704c458..6d1c247c821c 100644
--- a/arch/x86/lib/memset_64.S
+++ b/arch/x86/lib/memset_64.S
@@ -40,7 +40,11 @@ SYM_FUNC_START(__memset)
SYM_FUNC_END(__memset)
EXPORT_SYMBOL(__memset)

+#ifdef CONFIG_UML
+SYM_FUNC_ALIAS_WEAK(memset, __memset)
+#else
SYM_FUNC_ALIAS(memset, __memset)
+#endif
EXPORT_SYMBOL(memset)

SYM_FUNC_START_LOCAL(memset_orig)

---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230609-uml-kasan-2392dd4c3858

Best regards,
--
Vincent Whitchurch <[email protected]>


2023-09-15 15:49:28

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] x86: Fix build of UML with KASAN

On Fri, 2023-09-15 at 11:32 +0200, Ingo Molnar wrote:
>
> > ld: mm/kasan/shadow.o: in function `memset':
> > shadow.c:(.text+0x40): multiple definition of `memset';
> > arch/x86/lib/memset_64.o:(.noinstr.text+0x0): first defined here
> > ld: mm/kasan/shadow.o: in function `memmove':
> > shadow.c:(.text+0x90): multiple definition of `memmove';
> > arch/x86/lib/memmove_64.o:(.noinstr.text+0x0): first defined here
> > ld: mm/kasan/shadow.o: in function `memcpy':
> > shadow.c:(.text+0x110): multiple definition of `memcpy';
> > arch/x86/lib/memcpy_64.o:(.noinstr.text+0x0): first defined here
>
> So the breakage was ~9 months ago, and apparently nobody build-tested UML?

Well, first of all, it's only with KASAN, and then I think we probably
all did and applied a similar fix or this one ... I have a in my tree
that simplies marks the three symbols as weak again, for instance,
dating back to March 27th. Didn't publish it at the time, it probably
got lost in the shuffle, don't remember.


Also, a variant of this patch has been around for three months too.

> Does UML boot with the fix?

Sure, works fine as long as the symbols are marked weak _somehow_.

johannes

2023-09-15 16:58:02

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2] x86: Fix build of UML with KASAN


* Vincent Whitchurch <[email protected]> wrote:

> Building UML with KASAN fails since commit 69d4c0d32186 ("entry, kasan,
> x86: Disallow overriding mem*() functions") with the following errors:
>
> $ tools/testing/kunit/kunit.py run --kconfig_add CONFIG_KASAN=y
> ...
> ld: mm/kasan/shadow.o: in function `memset':
> shadow.c:(.text+0x40): multiple definition of `memset';
> arch/x86/lib/memset_64.o:(.noinstr.text+0x0): first defined here
> ld: mm/kasan/shadow.o: in function `memmove':
> shadow.c:(.text+0x90): multiple definition of `memmove';
> arch/x86/lib/memmove_64.o:(.noinstr.text+0x0): first defined here
> ld: mm/kasan/shadow.o: in function `memcpy':
> shadow.c:(.text+0x110): multiple definition of `memcpy';
> arch/x86/lib/memcpy_64.o:(.noinstr.text+0x0): first defined here

So the breakage was ~9 months ago, and apparently nobody build-tested UML?

Does UML boot with the fix?

> UML does not use GENERIC_ENTRY and is still supposed to be allowed to
> override the mem*() functions, so use weak aliases in that case.
>
> Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> Signed-off-by: Vincent Whitchurch <[email protected]>
> ---
> Changes in v2:
> - Use CONFIG_UML instead of CONFIG_GENERIC_ENTRY.
> - Link to v1: https://lore.kernel.org/r/[email protected]
> ---
> arch/x86/lib/memcpy_64.S | 4 ++++
> arch/x86/lib/memmove_64.S | 4 ++++
> arch/x86/lib/memset_64.S | 4 ++++
> 3 files changed, 12 insertions(+)
>
> diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
> index 8f95fb267caa..47b004851cf3 100644
> --- a/arch/x86/lib/memcpy_64.S
> +++ b/arch/x86/lib/memcpy_64.S
> @@ -40,7 +40,11 @@ SYM_TYPED_FUNC_START(__memcpy)
> SYM_FUNC_END(__memcpy)
> EXPORT_SYMBOL(__memcpy)
>
> +#ifdef CONFIG_UML
> +SYM_FUNC_ALIAS_WEAK(memcpy, __memcpy)
> +#else
> SYM_FUNC_ALIAS(memcpy, __memcpy)
> +#endif
> EXPORT_SYMBOL(memcpy)

Meh, the extra 3 #ifdefs are rather ugly and don't really express UML's
expectations here.

So how about introducing a SYM_FUNC_ALIAS_MEMFUNC() variant on x86 in a
suitable header, which maps to the right thing, with a comment added that
explains that this is for UML's mem*() functions?

Thanks,

Ingo