Re: [PATCH 12/13] powerpc/string: Add KMSAN support

[Christophe Leroy]

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 ?

> +#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 ?

> +#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)