diff mbox series

[-tip,v4,2/3] kasan: Treat meminstrinsic as builtins in uninstrumented files

Message ID 20230216234522.3757369-2-elver@google.com (mailing list archive)
State New, archived
Headers show
Series [-tip,v4,1/3] kasan: Emit different calls for instrumentable memintrinsics | expand

Commit Message

Marco Elver Feb. 16, 2023, 11:45 p.m. UTC
Where the compiler instruments meminstrinsics by generating calls to
__asan/__hwasan_ prefixed functions, let the compiler consider
memintrinsics as builtin again.

To do so, never override memset/memmove/memcpy if the compiler does the
correct instrumentation - even on !GENERIC_ENTRY architectures.

Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
Signed-off-by: Marco Elver <elver@google.com>
---
v4:
* New patch.
---
 lib/Kconfig.kasan      | 9 +++++++++
 mm/kasan/shadow.c      | 5 ++++-
 scripts/Makefile.kasan | 9 +++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

Comments

Andrey Konovalov Feb. 17, 2023, 11:07 a.m. UTC | #1
On Fri, Feb 17, 2023 at 12:45 AM Marco Elver <elver@google.com> wrote:
>
> Where the compiler instruments meminstrinsics by generating calls to
> __asan/__hwasan_ prefixed functions, let the compiler consider
> memintrinsics as builtin again.
>
> To do so, never override memset/memmove/memcpy if the compiler does the
> correct instrumentation - even on !GENERIC_ENTRY architectures.
>
> Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> v4:
> * New patch.
> ---
>  lib/Kconfig.kasan      | 9 +++++++++
>  mm/kasan/shadow.c      | 5 ++++-
>  scripts/Makefile.kasan | 9 +++++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index be6ee6020290..fdca89c05745 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -49,6 +49,15 @@ menuconfig KASAN
>
>  if KASAN
>
> +config CC_HAS_KASAN_MEMINTRINSIC_PREFIX
> +       def_bool (CC_IS_CLANG && $(cc-option,-fsanitize=kernel-address -mllvm -asan-kernel-mem-intrinsic-prefix=1)) || \
> +                (CC_IS_GCC && $(cc-option,-fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1))
> +       # Don't define it if we don't need it: compilation of the test uses
> +       # this variable to decide how the compiler should treat builtins.
> +       depends on !KASAN_HW_TAGS
> +       help
> +         The compiler is able to prefix memintrinsics with __asan or __hwasan.
> +
>  choice
>         prompt "KASAN mode"
>         default KASAN_GENERIC
> diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
> index f8a47cb299cb..43b6a59c8b54 100644
> --- a/mm/kasan/shadow.c
> +++ b/mm/kasan/shadow.c
> @@ -38,11 +38,14 @@ bool __kasan_check_write(const volatile void *p, unsigned int size)
>  }
>  EXPORT_SYMBOL(__kasan_check_write);
>
> -#ifndef CONFIG_GENERIC_ENTRY
> +#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY)
>  /*
>   * CONFIG_GENERIC_ENTRY relies on compiler emitted mem*() calls to not be
>   * instrumented. KASAN enabled toolchains should emit __asan_mem*() functions
>   * for the sites they want to instrument.
> + *
> + * If we have a compiler that can instrument meminstrinsics, never override
> + * these, so that non-instrumented files can safely consider them as builtins.
>   */
>  #undef memset
>  void *memset(void *addr, int c, size_t len)
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index fa9f836f8039..c186110ffa20 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -1,5 +1,14 @@
>  # SPDX-License-Identifier: GPL-2.0
> +
> +ifdef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX
> +# Safe for compiler to generate meminstrinsic calls in uninstrumented files.
> +CFLAGS_KASAN_NOSANITIZE :=
> +else
> +# Don't let compiler generate memintrinsic calls in uninstrumented files
> +# because they are instrumented.
>  CFLAGS_KASAN_NOSANITIZE := -fno-builtin
> +endif
> +
>  KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
>
>  cc-param = $(call cc-option, -mllvm -$(1), $(call cc-option, --param $(1)))
> --
> 2.39.2.637.g21b0678d19-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>

Is it also safe to remove custom mem* definitions from
arch/x86/include/asm/string_64.h now?

https://elixir.bootlin.com/linux/v6.2-rc8/source/arch/x86/include/asm/string_64.h#L88
Marco Elver Feb. 17, 2023, 12:55 p.m. UTC | #2
On Fri, 17 Feb 2023 at 12:07, Andrey Konovalov <andreyknvl@gmail.com> wrote:

> Is it also safe to remove custom mem* definitions from
> arch/x86/include/asm/string_64.h now?
>
> https://elixir.bootlin.com/linux/v6.2-rc8/source/arch/x86/include/asm/string_64.h#L88

Yes, I think so - sent another patch:
https://lore.kernel.org/all/Y+94tm7xoeTGqPgs@elver.google.com/
diff mbox series

Patch

diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index be6ee6020290..fdca89c05745 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -49,6 +49,15 @@  menuconfig KASAN
 
 if KASAN
 
+config CC_HAS_KASAN_MEMINTRINSIC_PREFIX
+	def_bool (CC_IS_CLANG && $(cc-option,-fsanitize=kernel-address -mllvm -asan-kernel-mem-intrinsic-prefix=1)) || \
+		 (CC_IS_GCC && $(cc-option,-fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1))
+	# Don't define it if we don't need it: compilation of the test uses
+	# this variable to decide how the compiler should treat builtins.
+	depends on !KASAN_HW_TAGS
+	help
+	  The compiler is able to prefix memintrinsics with __asan or __hwasan.
+
 choice
 	prompt "KASAN mode"
 	default KASAN_GENERIC
diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
index f8a47cb299cb..43b6a59c8b54 100644
--- a/mm/kasan/shadow.c
+++ b/mm/kasan/shadow.c
@@ -38,11 +38,14 @@  bool __kasan_check_write(const volatile void *p, unsigned int size)
 }
 EXPORT_SYMBOL(__kasan_check_write);
 
-#ifndef CONFIG_GENERIC_ENTRY
+#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY)
 /*
  * CONFIG_GENERIC_ENTRY relies on compiler emitted mem*() calls to not be
  * instrumented. KASAN enabled toolchains should emit __asan_mem*() functions
  * for the sites they want to instrument.
+ *
+ * If we have a compiler that can instrument meminstrinsics, never override
+ * these, so that non-instrumented files can safely consider them as builtins.
  */
 #undef memset
 void *memset(void *addr, int c, size_t len)
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index fa9f836f8039..c186110ffa20 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -1,5 +1,14 @@ 
 # SPDX-License-Identifier: GPL-2.0
+
+ifdef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX
+# Safe for compiler to generate meminstrinsic calls in uninstrumented files.
+CFLAGS_KASAN_NOSANITIZE :=
+else
+# Don't let compiler generate memintrinsic calls in uninstrumented files
+# because they are instrumented.
 CFLAGS_KASAN_NOSANITIZE := -fno-builtin
+endif
+
 KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
 
 cc-param = $(call cc-option, -mllvm -$(1), $(call cc-option, --param $(1)))