diff mbox series

[v5,1/4] kasan: Emit different calls for instrumentable memintrinsics

Message ID 20230224085942.1791837-1-elver@google.com (mailing list archive)
State Mainlined
Commit 51287dcb00cc715c27bf6a6b4dbd431621c5b65a
Headers show
Series [v5,1/4] kasan: Emit different calls for instrumentable memintrinsics | expand

Commit Message

Marco Elver Feb. 24, 2023, 8:59 a.m. UTC
Clang 15 provides an option to prefix memcpy/memset/memmove calls with
__asan_/__hwasan_ in instrumented functions: https://reviews.llvm.org/D122724

GCC will add support in future:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108777

Use it to regain KASAN instrumentation of memcpy/memset/memmove on
architectures that require noinstr to be really free from instrumented
mem*() functions (all GENERIC_ENTRY architectures).

Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
Signed-off-by: Marco Elver <elver@google.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
---
v4:
* Also enable it for KASAN_SW_TAGS (__hwasan_mem*).

v3:
* No change.

v2:
* Use asan-kernel-mem-intrinsic-prefix=1, so that once GCC supports the
  param, it also works there (it needs the =1).

The Fixes tag is just there to show the dependency, and that people
shouldn't apply this patch without 69d4c0d32186.
---
 mm/kasan/kasan.h       |  4 ++++
 mm/kasan/shadow.c      | 11 +++++++++++
 scripts/Makefile.kasan |  8 ++++++++
 3 files changed, 23 insertions(+)

Comments

Catalin Marinas April 13, 2023, 2:14 p.m. UTC | #1
Hi Marco,

On Fri, Feb 24, 2023 at 09:59:39AM +0100, Marco Elver wrote:
> Clang 15 provides an option to prefix memcpy/memset/memmove calls with
> __asan_/__hwasan_ in instrumented functions: https://reviews.llvm.org/D122724
> 
> GCC will add support in future:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108777
> 
> Use it to regain KASAN instrumentation of memcpy/memset/memmove on
> architectures that require noinstr to be really free from instrumented
> mem*() functions (all GENERIC_ENTRY architectures).
> 
> Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> Signed-off-by: Marco Elver <elver@google.com>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
[...]
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index b9e94c5e7097..fa9f836f8039 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -38,6 +38,11 @@ endif
>  
>  CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
>  
> +# Instrument memcpy/memset/memmove calls by using instrumented __asan_mem*()
> +# instead. With compilers that don't support this option, compiler-inserted
> +# memintrinsics won't be checked by KASAN on GENERIC_ENTRY architectures.
> +CFLAGS_KASAN += $(call cc-param,asan-kernel-mem-intrinsic-prefix=1)
> +
>  endif # CONFIG_KASAN_GENERIC
>  
>  ifdef CONFIG_KASAN_SW_TAGS
> @@ -54,6 +59,9 @@ CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
>  		$(call cc-param,hwasan-inline-all-checks=0) \
>  		$(instrumentation_flags)
>  
> +# Instrument memcpy/memset/memmove calls by using instrumented __hwasan_mem*().
> +CFLAGS_KASAN += $(call cc-param,hwasan-kernel-mem-intrinsic-prefix=1)

This patch breaks the arm64 kernel builds with KASAN_SW_TAGS enabled and
clang prior to version 15. Those prior clang versions don't like the
'-mllvm -hwasan-kernel-mem-intrinsic-prefix=1' option, end up printing
the help text instead of generating the object.

Do we need some combination of cc-option and cc-param? Or at least
disable this instrumentation if earlier clang versions are used.

It's already in mainline as commit
51287dcb00cc715c27bf6a6b4dbd431621c5b65a.

Thanks.
Marco Elver April 18, 2023, 12:12 p.m. UTC | #2
On Thu, 13 Apr 2023 at 16:14, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> Hi Marco,
>
> On Fri, Feb 24, 2023 at 09:59:39AM +0100, Marco Elver wrote:
> > Clang 15 provides an option to prefix memcpy/memset/memmove calls with
> > __asan_/__hwasan_ in instrumented functions: https://reviews.llvm.org/D122724
> >
> > GCC will add support in future:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108777
> >
> > Use it to regain KASAN instrumentation of memcpy/memset/memmove on
> > architectures that require noinstr to be really free from instrumented
> > mem*() functions (all GENERIC_ENTRY architectures).
> >
> > Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> > Signed-off-by: Marco Elver <elver@google.com>
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
> > Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> [...]
> > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> > index b9e94c5e7097..fa9f836f8039 100644
> > --- a/scripts/Makefile.kasan
> > +++ b/scripts/Makefile.kasan
> > @@ -38,6 +38,11 @@ endif
> >
> >  CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
> >
> > +# Instrument memcpy/memset/memmove calls by using instrumented __asan_mem*()
> > +# instead. With compilers that don't support this option, compiler-inserted
> > +# memintrinsics won't be checked by KASAN on GENERIC_ENTRY architectures.
> > +CFLAGS_KASAN += $(call cc-param,asan-kernel-mem-intrinsic-prefix=1)
> > +
> >  endif # CONFIG_KASAN_GENERIC
> >
> >  ifdef CONFIG_KASAN_SW_TAGS
> > @@ -54,6 +59,9 @@ CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
> >               $(call cc-param,hwasan-inline-all-checks=0) \
> >               $(instrumentation_flags)
> >
> > +# Instrument memcpy/memset/memmove calls by using instrumented __hwasan_mem*().
> > +CFLAGS_KASAN += $(call cc-param,hwasan-kernel-mem-intrinsic-prefix=1)
>
> This patch breaks the arm64 kernel builds with KASAN_SW_TAGS enabled and
> clang prior to version 15. Those prior clang versions don't like the
> '-mllvm -hwasan-kernel-mem-intrinsic-prefix=1' option, end up printing
> the help text instead of generating the object.
>
> Do we need some combination of cc-option and cc-param? Or at least
> disable this instrumentation if earlier clang versions are used.
>
> It's already in mainline as commit
> 51287dcb00cc715c27bf6a6b4dbd431621c5b65a.

Arnd posted a patch, but the reason why a workaround is needed is
quite unfortunate:
https://lore.kernel.org/all/CANpmjNMwYosrvqh4ogDO8rgn+SeDHM2b-shD21wTypm_6MMe=g@mail.gmail.com/

Clang apparently interprets unknown options that start with "-h..",
i.e. "-mllvm -h..." as a request to print help text, which has exit
code 0. So this is only a problem for hwasan options.
diff mbox series

Patch

diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 71c15438afcf..172713b87556 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -637,4 +637,8 @@  void __hwasan_storeN_noabort(unsigned long addr, size_t size);
 
 void __hwasan_tag_memory(unsigned long addr, u8 tag, unsigned long size);
 
+void *__hwasan_memset(void *addr, int c, size_t len);
+void *__hwasan_memmove(void *dest, const void *src, size_t len);
+void *__hwasan_memcpy(void *dest, const void *src, size_t len);
+
 #endif /* __MM_KASAN_KASAN_H */
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index 98269936a5e4..f8a47cb299cb 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -107,6 +107,17 @@  void *__asan_memcpy(void *dest, const void *src, size_t len)
 }
 EXPORT_SYMBOL(__asan_memcpy);
 
+#ifdef CONFIG_KASAN_SW_TAGS
+void *__hwasan_memset(void *addr, int c, size_t len) __alias(__asan_memset);
+EXPORT_SYMBOL(__hwasan_memset);
+#ifdef __HAVE_ARCH_MEMMOVE
+void *__hwasan_memmove(void *dest, const void *src, size_t len) __alias(__asan_memmove);
+EXPORT_SYMBOL(__hwasan_memmove);
+#endif
+void *__hwasan_memcpy(void *dest, const void *src, size_t len) __alias(__asan_memcpy);
+EXPORT_SYMBOL(__hwasan_memcpy);
+#endif
+
 void kasan_poison(const void *addr, size_t size, u8 value, bool init)
 {
 	void *shadow_start, *shadow_end;
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index b9e94c5e7097..fa9f836f8039 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -38,6 +38,11 @@  endif
 
 CFLAGS_KASAN += $(call cc-param,asan-stack=$(stack_enable))
 
+# Instrument memcpy/memset/memmove calls by using instrumented __asan_mem*()
+# instead. With compilers that don't support this option, compiler-inserted
+# memintrinsics won't be checked by KASAN on GENERIC_ENTRY architectures.
+CFLAGS_KASAN += $(call cc-param,asan-kernel-mem-intrinsic-prefix=1)
+
 endif # CONFIG_KASAN_GENERIC
 
 ifdef CONFIG_KASAN_SW_TAGS
@@ -54,6 +59,9 @@  CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
 		$(call cc-param,hwasan-inline-all-checks=0) \
 		$(instrumentation_flags)
 
+# Instrument memcpy/memset/memmove calls by using instrumented __hwasan_mem*().
+CFLAGS_KASAN += $(call cc-param,hwasan-kernel-mem-intrinsic-prefix=1)
+
 endif # CONFIG_KASAN_SW_TAGS
 
 export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE