Re: [PATCH 12/13] powerpc/string: Add KMSAN support
From: Nicholas Miehlbradt
Date: Tue Jan 09 2024 - 23:09:52 EST
On 14/12/2023 8:25 pm, Christophe Leroy wrote:
Le 14/12/2023 à 06:55, Nicholas Miehlbradt a écrit :
KMSAN expects functions __mem{set,cpy,move} so add aliases pointing to
the respective functions.
Disable use of architecture specific memset{16,32,64} to ensure that
metadata is correctly updated and strn{cpy,cmp} and mem{chr,cmp} which
are implemented in assembly and therefore cannot be instrumented to
propagate/check metadata.
Alias calls to mem{set,cpy,move} to __msan_mem{set,cpy,move} in
instrumented code to correctly propagate metadata.
Signed-off-by: Nicholas Miehlbradt <nicholas@xxxxxxxxxxxxx>
---
arch/powerpc/include/asm/kmsan.h | 7 +++++++
arch/powerpc/include/asm/string.h | 18 ++++++++++++++++--
arch/powerpc/lib/Makefile | 2 ++
arch/powerpc/lib/mem_64.S | 5 ++++-
arch/powerpc/lib/memcpy_64.S | 2 ++
.../selftests/powerpc/copyloops/asm/kmsan.h | 0
.../selftests/powerpc/copyloops/linux/export.h | 1 +
7 files changed, 32 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/copyloops/asm/kmsan.h
diff --git a/arch/powerpc/include/asm/kmsan.h b/arch/powerpc/include/asm/kmsan.h
index bc84f6ff2ee9..fc59dc24e170 100644
--- a/arch/powerpc/include/asm/kmsan.h
+++ b/arch/powerpc/include/asm/kmsan.h
@@ -7,6 +7,13 @@
#ifndef _ASM_POWERPC_KMSAN_H
#define _ASM_POWERPC_KMSAN_H
+#ifdef CONFIG_KMSAN
+#define EXPORT_SYMBOL_KMSAN(fn) SYM_FUNC_ALIAS(__##fn, fn) \
+ EXPORT_SYMBOL(__##fn)
+#else
+#define EXPORT_SYMBOL_KMSAN(fn)
+#endif
+
#ifndef __ASSEMBLY__
#ifndef MODULE
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 60ba22770f51..412626ce619b 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -4,7 +4,7 @@
#ifdef __KERNEL__
-#ifndef CONFIG_KASAN
+#if !defined(CONFIG_KASAN) && !defined(CONFIG_KMSAN)
#define __HAVE_ARCH_STRNCPY
#define __HAVE_ARCH_STRNCMP
#define __HAVE_ARCH_MEMCHR
@@ -56,8 +56,22 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
#endif /* CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX */
#endif /* CONFIG_KASAN */
+#ifdef CONFIG_KMSAN
+
+void *__memset(void *s, int c, __kernel_size_t count);
+void *__memcpy(void *to, const void *from, __kernel_size_t n);
+void *__memmove(void *to, const void *from, __kernel_size_t n);
+
The same is done for KASAN, can't you reuse it ?
I tried this but I believe it makes the file more disorganised and
difficult to edit since there ends up being a set of definitions for
each intersection of features e.g. the definitions needed for both KASAN
and KMSAN, just KASAN, just KMSAN, etc.
This way it's clearer what each sanitizer needs and changing definitions
for one one sanitizer won't require refactors affecting other sanitizers.
+#ifdef __SANITIZE_MEMORY__
+#include <linux/kmsan_string.h>
+#define memset __msan_memset
+#define memcpy __msan_memcpy
+#define memmove __msan_memmove
+#endif
Will that work as you wish ?
What about the calls to memset() or memcpy() emited directly by GCC ?
These are handled by the compiler instrumentation which replaces these
with calls to the instrumented equivalent.
+#endif /* CONFIG_KMSAN */
+
#ifdef CONFIG_PPC64
-#ifndef CONFIG_KASAN
+#if !defined(CONFIG_KASAN) && !defined(CONFIG_KMSAN)
#define __HAVE_ARCH_MEMSET32
#define __HAVE_ARCH_MEMSET64
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 51ad0397c17a..fc3ea3eebbd6 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -32,9 +32,11 @@ obj-y += code-patching.o feature-fixups.o pmem.o
obj-$(CONFIG_CODE_PATCHING_SELFTEST) += test-code-patching.o
ifndef CONFIG_KASAN
+ifndef CONFIG_KMSAN
obj-y += string.o memcmp_$(BITS).o
obj-$(CONFIG_PPC32) += strlen_32.o
endif
+endif
obj-$(CONFIG_PPC32) += div64.o copy_32.o crtsavres.o
diff --git a/arch/powerpc/lib/mem_64.S b/arch/powerpc/lib/mem_64.S
index 6fd06cd20faa..a55f2fac49b3 100644
--- a/arch/powerpc/lib/mem_64.S
+++ b/arch/powerpc/lib/mem_64.S
@@ -9,8 +9,9 @@
#include <asm/errno.h>
#include <asm/ppc_asm.h>
#include <asm/kasan.h>
+#include <asm/kmsan.h>
-#ifndef CONFIG_KASAN
+#if !defined(CONFIG_KASAN) && !defined(CONFIG_KMSAN)
_GLOBAL(__memset16)
rlwimi r4,r4,16,0,15
/* fall through */
@@ -96,6 +97,7 @@ _GLOBAL_KASAN(memset)
blr
EXPORT_SYMBOL(memset)
EXPORT_SYMBOL_KASAN(memset)
+EXPORT_SYMBOL_KMSAN(memset)
_GLOBAL_TOC_KASAN(memmove)
cmplw 0,r3,r4
@@ -140,3 +142,4 @@ _GLOBAL(backwards_memcpy)
b 1b
EXPORT_SYMBOL(memmove)
EXPORT_SYMBOL_KASAN(memmove)
+EXPORT_SYMBOL_KMSAN(memmove)
diff --git a/arch/powerpc/lib/memcpy_64.S b/arch/powerpc/lib/memcpy_64.S
index b5a67e20143f..1657861618cc 100644
--- a/arch/powerpc/lib/memcpy_64.S
+++ b/arch/powerpc/lib/memcpy_64.S
@@ -8,6 +8,7 @@
#include <asm/asm-compat.h>
#include <asm/feature-fixups.h>
#include <asm/kasan.h>
+#include <asm/kmsan.h>
#ifndef SELFTEST_CASE
/* For big-endian, 0 == most CPUs, 1 == POWER6, 2 == Cell */
@@ -228,3 +229,4 @@ END_FTR_SECTION_IFCLR(CPU_FTR_UNALIGNED_LD_STD)
#endif
EXPORT_SYMBOL(memcpy)
EXPORT_SYMBOL_KASAN(memcpy)
+EXPORT_SYMBOL_KMSAN(memcpy)
diff --git a/tools/testing/selftests/powerpc/copyloops/asm/kmsan.h b/tools/testing/selftests/powerpc/copyloops/asm/kmsan.h
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tools/testing/selftests/powerpc/copyloops/linux/export.h b/tools/testing/selftests/powerpc/copyloops/linux/export.h
index e6b80d5fbd14..6379624bbf9b 100644
--- a/tools/testing/selftests/powerpc/copyloops/linux/export.h
+++ b/tools/testing/selftests/powerpc/copyloops/linux/export.h
@@ -2,3 +2,4 @@
#define EXPORT_SYMBOL(x)
#define EXPORT_SYMBOL_GPL(x)
#define EXPORT_SYMBOL_KASAN(x)
+#define EXPORT_SYMBOL_KMSAN(x)