2023-02-27 09:51:57

by Marco Elver

[permalink] [raw]
Subject: [PATCH mm] kasan, powerpc: Don't rename memintrinsics if compiler adds prefixes

With appropriate compiler support [1], KASAN builds use __asan prefixed
meminstrinsics, and KASAN no longer overrides memcpy/memset/memmove.

If compiler support is detected (CC_HAS_KASAN_MEMINTRINSIC_PREFIX),
define memintrinsics normally (do not prefix '__').

On powerpc, KASAN is the only user of __mem functions, which are used to
define instrumented memintrinsics. Alias the normal versions for KASAN
to use in its implementation.

Link: https://lore.kernel.org/all/[email protected]/ [1]
Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
arch/powerpc/include/asm/kasan.h | 2 +-
arch/powerpc/include/asm/string.h | 15 +++++++++++----
arch/powerpc/kernel/prom_init_check.sh | 9 +++++++--
3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
index 92a968202ba7..365d2720097c 100644
--- a/arch/powerpc/include/asm/kasan.h
+++ b/arch/powerpc/include/asm/kasan.h
@@ -2,7 +2,7 @@
#ifndef __ASM_KASAN_H
#define __ASM_KASAN_H

-#ifdef CONFIG_KASAN
+#if defined(CONFIG_KASAN) && !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX)
#define _GLOBAL_KASAN(fn) _GLOBAL(__##fn)
#define _GLOBAL_TOC_KASAN(fn) _GLOBAL_TOC(__##fn)
#define EXPORT_SYMBOL_KASAN(fn) EXPORT_SYMBOL(__##fn)
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 2aa0e31e6884..60ba22770f51 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -30,11 +30,17 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
extern void * memchr(const void *,int,__kernel_size_t);
void memcpy_flushcache(void *dest, const void *src, size_t size);

+#ifdef CONFIG_KASAN
+/* __mem variants are used by KASAN to implement instrumented meminstrinsics. */
+#ifdef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX
+#define __memset memset
+#define __memcpy memcpy
+#define __memmove memmove
+#else /* CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX */
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);
-
-#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+#ifndef __SANITIZE_ADDRESS__
/*
* For files that are not instrumented (e.g. mm/slub.c) we
* should use not instrumented version of mem* functions.
@@ -46,8 +52,9 @@ void *__memmove(void *to, const void *from, __kernel_size_t n);
#ifndef __NO_FORTIFY
#define __NO_FORTIFY /* FORTIFY_SOURCE uses __builtin_memcpy, etc. */
#endif
-
-#endif
+#endif /* !__SANITIZE_ADDRESS__ */
+#endif /* CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX */
+#endif /* CONFIG_KASAN */

#ifdef CONFIG_PPC64
#ifndef CONFIG_KASAN
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 311890d71c4c..f3f43a8f48cf 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -13,8 +13,13 @@
# If you really need to reference something from prom_init.o add
# it to the list below:

-grep "^CONFIG_KASAN=y$" ${KCONFIG_CONFIG} >/dev/null
-if [ $? -eq 0 ]
+has_renamed_memintrinsics()
+{
+ grep -q "^CONFIG_KASAN=y$" ${KCONFIG_CONFIG} && \
+ ! grep -q "^CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y" ${KCONFIG_CONFIG}
+}
+
+if has_renamed_memintrinsics
then
MEM_FUNCS="__memcpy __memset"
else
--
2.39.2.637.g21b0678d19-goog



2023-02-27 22:16:52

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH mm] kasan, powerpc: Don't rename memintrinsics if compiler adds prefixes

On Mon, 27 Feb 2023 10:47:27 +0100 Marco Elver <[email protected]> wrote:

> With appropriate compiler support [1], KASAN builds use __asan prefixed
> meminstrinsics, and KASAN no longer overrides memcpy/memset/memmove.
>
> If compiler support is detected (CC_HAS_KASAN_MEMINTRINSIC_PREFIX),
> define memintrinsics normally (do not prefix '__').
>
> On powerpc, KASAN is the only user of __mem functions, which are used to
> define instrumented memintrinsics. Alias the normal versions for KASAN
> to use in its implementation.
>
> Link: https://lore.kernel.org/all/[email protected]/ [1]
> Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>

Seems this is a fix against "kasan: treat meminstrinsic as builtins in
uninstrumented files", so I'll plan to fold this patch into that patch.


2023-02-27 23:10:04

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH mm] kasan, powerpc: Don't rename memintrinsics if compiler adds prefixes

On Mon, 27 Feb 2023 at 23:16, Andrew Morton <[email protected]> wrote:
>
> On Mon, 27 Feb 2023 10:47:27 +0100 Marco Elver <[email protected]> wrote:
>
> > With appropriate compiler support [1], KASAN builds use __asan prefixed
> > meminstrinsics, and KASAN no longer overrides memcpy/memset/memmove.
> >
> > If compiler support is detected (CC_HAS_KASAN_MEMINTRINSIC_PREFIX),
> > define memintrinsics normally (do not prefix '__').
> >
> > On powerpc, KASAN is the only user of __mem functions, which are used to
> > define instrumented memintrinsics. Alias the normal versions for KASAN
> > to use in its implementation.
> >
> > Link: https://lore.kernel.org/all/[email protected]/ [1]
> > Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> > Reported-by: kernel test robot <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
>
> Seems this is a fix against "kasan: treat meminstrinsic as builtins in
> uninstrumented files", so I'll plan to fold this patch into that patch.

Yes, that looks right.

If a powerpc maintainer could take a quick look as well would be good.
The maze of memcpy/memmove/memset definitions and redefinitions isn't
the simplest - I hope in a few years we can delete all the old code
(before CC_HAS_KASAN_MEMINTRINSIC_PREFIX), and let the compilers just
"do the right thing".

Thanks,
-- Marco

2023-02-28 11:58:14

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH mm] kasan, powerpc: Don't rename memintrinsics if compiler adds prefixes

Marco Elver <[email protected]> writes:
> On Mon, 27 Feb 2023 at 23:16, Andrew Morton <[email protected]> wrote:
>>
>> On Mon, 27 Feb 2023 10:47:27 +0100 Marco Elver <[email protected]> wrote:
>>
>> > With appropriate compiler support [1], KASAN builds use __asan prefixed
>> > meminstrinsics, and KASAN no longer overrides memcpy/memset/memmove.
>> >
>> > If compiler support is detected (CC_HAS_KASAN_MEMINTRINSIC_PREFIX),
>> > define memintrinsics normally (do not prefix '__').
>> >
>> > On powerpc, KASAN is the only user of __mem functions, which are used to
>> > define instrumented memintrinsics. Alias the normal versions for KASAN
>> > to use in its implementation.
>> >
>> > Link: https://lore.kernel.org/all/[email protected]/ [1]
>> > Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>> > Reported-by: kernel test robot <[email protected]>
>> > Signed-off-by: Marco Elver <[email protected]>
>>
>> Seems this is a fix against "kasan: treat meminstrinsic as builtins in
>> uninstrumented files", so I'll plan to fold this patch into that patch.
>
> Yes, that looks right.
>
> If a powerpc maintainer could take a quick look as well would be good.

The patch looks OK to me. It builds for various configs and I did a few
test boots with KASAN enabled, everything seems normal.

Acked-by: Michael Ellerman <[email protected]> (powerpc)


> The maze of memcpy/memmove/memset definitions and redefinitions isn't
> the simplest - I hope in a few years we can delete all the old code
> (before CC_HAS_KASAN_MEMINTRINSIC_PREFIX), and let the compilers just
> "do the right thing".

Yeah that would be nice.

cheers