From: Nicholas Miehlbradt <nicholas@linux.ibm.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
"glider@google.com" <glider@google.com>,
"elver@google.com" <elver@google.com>,
"dvyukov@google.com" <dvyukov@google.com>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
"mpe@ellerman.id.au" <mpe@ellerman.id.au>,
"npiggin@gmail.com" <npiggin@gmail.com>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"iii@linux.ibm.com" <iii@linux.ibm.com>,
"kasan-dev@googlegroups.com" <kasan-dev@googlegroups.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 12/13] powerpc/string: Add KMSAN support
Date: Wed, 10 Jan 2024 15:09:17 +1100 [thread overview]
Message-ID: <55c57f88-9975-4510-b6bc-7e78462e0a62@linux.ibm.com> (raw)
In-Reply-To: <2f35548a-bdbd-4c37-8f60-cebeb381a7af@csgroup.eu>
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@linux.ibm.com>
>> ---
>> 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)
next prev parent reply other threads:[~2024-01-10 4:10 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-14 5:55 [PATCH 00/13] kmsan: Enable on powerpc Nicholas Miehlbradt
2023-12-14 5:55 ` [PATCH 01/13] kmsan: Export kmsan_handle_dma Nicholas Miehlbradt
2024-02-19 19:37 ` Christophe Leroy
2023-12-14 5:55 ` [PATCH 02/13] hvc: Fix use of uninitialized array in udbg_hvc_putc Nicholas Miehlbradt
2023-12-14 8:36 ` Christophe Leroy
2023-12-21 12:09 ` Michael Ellerman
2023-12-14 5:55 ` [PATCH 03/13] powerpc: Disable KMSAN santitization for prom_init, vdso and purgatory Nicholas Miehlbradt
2023-12-14 5:55 ` [PATCH 04/13] powerpc: Disable CONFIG_DCACHE_WORD_ACCESS when KMSAN is enabled Nicholas Miehlbradt
2023-12-14 8:42 ` Christophe Leroy
2023-12-14 5:55 ` [PATCH 05/13] powerpc: Unpoison buffers populated by hcalls Nicholas Miehlbradt
2023-12-14 5:55 ` [PATCH 06/13] powerpc/pseries/nvram: Unpoison buffer populated by rtas_call Nicholas Miehlbradt
2023-12-14 5:55 ` [PATCH 07/13] powerpc/kprobes: Unpoison instruction in kprobe struct Nicholas Miehlbradt
2023-12-15 7:51 ` Naveen N Rao
2023-12-14 5:55 ` [PATCH 08/13] powerpc: Unpoison pt_regs Nicholas Miehlbradt
2023-12-14 5:55 ` [PATCH 09/13] powerpc: Disable KMSAN checks on functions which walk the stack Nicholas Miehlbradt
2023-12-14 9:00 ` Christophe Leroy
2024-01-10 4:16 ` Nicholas Miehlbradt
2023-12-15 9:02 ` Aneesh Kumar K.V
2023-12-14 5:55 ` [PATCH 10/13] powerpc: Define KMSAN metadata address ranges for vmalloc and ioremap Nicholas Miehlbradt
2023-12-14 9:17 ` Christophe Leroy
2024-01-10 3:54 ` Nicholas Miehlbradt
2023-12-15 9:27 ` Aneesh Kumar K.V
2023-12-14 5:55 ` [PATCH 11/13] powerpc: Implement architecture specific KMSAN interface Nicholas Miehlbradt
2023-12-14 9:20 ` Christophe Leroy
2023-12-14 5:55 ` [PATCH 12/13] powerpc/string: Add KMSAN support Nicholas Miehlbradt
2023-12-14 9:25 ` Christophe Leroy
2024-01-10 4:09 ` Nicholas Miehlbradt [this message]
2023-12-14 5:55 ` [PATCH 13/13] powerpc: Enable KMSAN on powerpc Nicholas Miehlbradt
2023-12-14 9:27 ` Christophe Leroy
2024-02-20 6:39 ` [PATCH 00/13] kmsan: Enable " Christophe Leroy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55c57f88-9975-4510-b6bc-7e78462e0a62@linux.ibm.com \
--to=nicholas@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=christophe.leroy@csgroup.eu \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=glider@google.com \
--cc=iii@linux.ibm.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mpe@ellerman.id.au \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).