linux-kernel - Re: [PATCH -tip v4 2/3] kasan: Treat meminstrinsic as builtins in uninstrumented files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+fCnZehvF1o4rQJah=SXaS-AXWs--h2CDaUca-hJK=ZTD8kTg@mail.gmail.com>
Date:   Fri, 17 Feb 2023 12:07:20 +0100
From:   Andrey Konovalov <andreyknvl@...il.com>
To:     Marco Elver <elver@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Jakub Jelinek <jakub@...hat.com>,
        linux-toolchains@...r.kernel.org,
        Andrey Ryabinin <ryabinin.a.a@...il.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH -tip v4 2/3] kasan: Treat meminstrinsic as builtins in
 uninstrumented files

On Fri, Feb 17, 2023 at 12:45 AM Marco Elver <elver@...gle.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@...gle.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@...il.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