[PATCH] x86: Fix build of UML with KASAN

[Vincent Whitchurch]

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

If I'm reading that commit right, the !GENERIC_ENTRY case 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 <vincent.w...@axis.com>
---
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 8f95fb267caa7..5dc265b36ef0b 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_GENERIC_ENTRY
SYM_FUNC_ALIAS(memcpy, __memcpy)
+#else
+SYM_FUNC_ALIAS_WEAK(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 02661861e5dd9..3b1a02357fb29 100644
--- a/arch/x86/lib/memmove_64.S
+++ b/arch/x86/lib/memmove_64.S
@@ -215,5 +215,9 @@ SYM_FUNC_START(__memmove)
SYM_FUNC_END(__memmove)
EXPORT_SYMBOL(__memmove)

+#ifdef CONFIG_GENERIC_ENTRY
SYM_FUNC_ALIAS(memmove, __memmove)
+#else
+SYM_FUNC_ALIAS_WEAK(memmove, __memmove)
+#endif
EXPORT_SYMBOL(memmove)
diff --git a/arch/x86/lib/memset_64.S b/arch/x86/lib/memset_64.S
index 7c59a704c4584..fe27538a355db 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_GENERIC_ENTRY
SYM_FUNC_ALIAS(memset, __memset)
+#else
+SYM_FUNC_ALIAS_WEAK(memset, __memset)
+#endif
EXPORT_SYMBOL(memset)

SYM_FUNC_START_LOCAL(memset_orig)

---
base-commit: 9561de3a55bed6bdd44a12820ba81ec416e705a7
change-id: 20230609-uml-kasan-2392dd4c3858

Best regards,
--
Vincent Whitchurch <vincent.w...@axis.com>

[David Gow]

On Fri, 9 Jun 2023 at 19:19, Vincent Whitchurch
<vincent.w...@axis.com> 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
>
> If I'm reading that commit right, the !GENERIC_ENTRY case 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 <vincent.w...@axis.com>
> ---

Thanks: I stumbled into this the other day and ran out of time to debug it.

I've tested that it works here.

Tested-by: David Gow <davi...@google.com>

Cheers,
-- David

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20230609-uml-kasan-v1-1-5fac8d409d4f%40axis.com.

[Vincent Whitchurch]

Thanks. Perhaps someone could pick this up? It's been a few months,
and the build problem is still present on v6.6-rc1.

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Vincent Whitchurch" <Vincent.W...@axis.com>

>> Thanks: I stumbled into this the other day and ran out of time to debug it.
>>
>> I've tested that it works here.
>>
>> Tested-by: David Gow <davi...@google.com>
>
> Thanks. Perhaps someone could pick this up? It's been a few months,
> and the build problem is still present on v6.6-rc1.

I'll happily carry it though the UML tree if we get an ACK from x86 maintainers.

Thanks,
//richard

[Peter Zijlstra]

Urgh...

Can we use CONFIG_UML here to clarify things? It's a bit of a bother UML
is diverging to much, but oh well.

[Vincent Whitchurch]

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 <vincent.w...@axis.com>
---

Changes in v2:
- Use CONFIG_UML instead of CONFIG_GENERIC_ENTRY.
- Link to v1: https://lore.kernel.org/r/20230609-uml-kasan...@axis.com

---
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

[Ingo Molnar]

* Vincent Whitchurch <vincent.w...@axis.com> 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?

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

[Johannes Berg]

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

[Vincent Whitchurch]

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 <vincent.w...@axis.com>
---

Changes in v3:
- Add SYM_FUNC_ALIAS_MEMFUNC() macro to avoid ifdefs in multiple places.
- Link to v2: https://lore.kernel.org/r/20230915-uml-kasan...@axis.com

Changes in v2:
- Use CONFIG_UML instead of CONFIG_GENERIC_ENTRY.
- Link to v1: https://lore.kernel.org/r/20230609-uml-kasan...@axis.com
---

arch/x86/include/asm/linkage.h | 7 +++++++
arch/x86/lib/memcpy_64.S | 2 +-
arch/x86/lib/memmove_64.S | 2 +-
arch/x86/lib/memset_64.S | 2 +-
4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/linkage.h b/arch/x86/include/asm/linkage.h
index 97a3de7892d3..32cdf1e92cfb 100644
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -97,6 +97,13 @@
CFI_POST_PADDING \
SYM_FUNC_END(__cfi_##name)

+/* UML needs to be able to override memcpy() and friends for KASAN. */
+#ifdef CONFIG_UML
+#define SYM_FUNC_ALIAS_MEMFUNC SYM_FUNC_ALIAS_WEAK
+#else
+#define SYM_FUNC_ALIAS_MEMFUNC SYM_FUNC_ALIAS
+#endif
+
/* SYM_TYPED_FUNC_START -- use for indirectly called globals, w/ CFI type */
#define SYM_TYPED_FUNC_START(name) \
SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_F_ALIGN) \
diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 8f95fb267caa..76697df8dfd5 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -40,7 +40,7 @@ SYM_TYPED_FUNC_START(__memcpy)
SYM_FUNC_END(__memcpy)
EXPORT_SYMBOL(__memcpy)

-SYM_FUNC_ALIAS(memcpy, __memcpy)
+SYM_FUNC_ALIAS_MEMFUNC(memcpy, __memcpy)

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..ccdf3a597045 100644
--- a/arch/x86/lib/memmove_64.S
+++ b/arch/x86/lib/memmove_64.S
@@ -212,5 +212,5 @@ SYM_FUNC_START(__memmove)
SYM_FUNC_END(__memmove)
EXPORT_SYMBOL(__memmove)

-SYM_FUNC_ALIAS(memmove, __memmove)
+SYM_FUNC_ALIAS_MEMFUNC(memmove, __memmove)

EXPORT_SYMBOL(memmove)
diff --git a/arch/x86/lib/memset_64.S b/arch/x86/lib/memset_64.S

index 7c59a704c458..3d818b849ec6 100644
--- a/arch/x86/lib/memset_64.S
+++ b/arch/x86/lib/memset_64.S
@@ -40,7 +40,7 @@ SYM_FUNC_START(__memset)
SYM_FUNC_END(__memset)
EXPORT_SYMBOL(__memset)

-SYM_FUNC_ALIAS(memset, __memset)
+SYM_FUNC_ALIAS_MEMFUNC(memset, __memset)