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+fCnZdsiWjpp9qjsy16SSuOcaOgnk2h6vC+dq6h8GUrqdF1bw@mail.gmail.com>
Date:   Fri, 17 Feb 2023 12:07:28 +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,
        Linux Kernel Functional Testing <lkft@...aro.org>
Subject: Re: [PATCH -tip v4 3/3] kasan: test: Fix test for new meminstrinsic instrumentation

On Fri, Feb 17, 2023 at 12:45 AM Marco Elver <elver@...gle.com> wrote:
>
> The tests for memset/memmove have been failing since they haven't been
> instrumented in 69d4c0d32186.
>
> Fix the test to recognize when memintrinsics aren't instrumented, and
> skip test cases accordingly. We also need to conditionally pass
> -fno-builtin to the test, otherwise the instrumentation pass won't
> recognize memintrinsics and end up not instrumenting them either.
>
> Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> Reported-by: Linux Kernel Functional Testing <lkft@...aro.org>
> Signed-off-by: Marco Elver <elver@...gle.com>
> ---
> v4:
> * New patch.
> ---
>  mm/kasan/Makefile     |  9 ++++++++-
>  mm/kasan/kasan_test.c | 29 +++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/mm/kasan/Makefile b/mm/kasan/Makefile
> index d4837bff3b60..7634dd2a6128 100644
> --- a/mm/kasan/Makefile
> +++ b/mm/kasan/Makefile
> @@ -35,7 +35,14 @@ CFLAGS_shadow.o := $(CC_FLAGS_KASAN_RUNTIME)
>  CFLAGS_hw_tags.o := $(CC_FLAGS_KASAN_RUNTIME)
>  CFLAGS_sw_tags.o := $(CC_FLAGS_KASAN_RUNTIME)
>
> -CFLAGS_KASAN_TEST := $(CFLAGS_KASAN) -fno-builtin $(call cc-disable-warning, vla)
> +CFLAGS_KASAN_TEST := $(CFLAGS_KASAN) $(call cc-disable-warning, vla)
> +ifndef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX
> +# If compiler instruments memintrinsics by prefixing them with __asan/__hwasan,
> +# we need to treat them normally (as builtins), otherwise the compiler won't
> +# recognize them as instrumentable. If it doesn't instrument them, we need to
> +# pass -fno-builtin, so the compiler doesn't inline them.
> +CFLAGS_KASAN_TEST += -fno-builtin
> +endif
>
>  CFLAGS_kasan_test.o := $(CFLAGS_KASAN_TEST)
>  CFLAGS_kasan_test_module.o := $(CFLAGS_KASAN_TEST)
> diff --git a/mm/kasan/kasan_test.c b/mm/kasan/kasan_test.c
> index 74cd80c12b25..627eaf1ee1db 100644
> --- a/mm/kasan/kasan_test.c
> +++ b/mm/kasan/kasan_test.c
> @@ -165,6 +165,15 @@ static void kasan_test_exit(struct kunit *test)
>                 kunit_skip((test), "Test requires " #config "=n");      \
>  } while (0)
>
> +#define KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test) do {              \
> +       if (IS_ENABLED(CONFIG_KASAN_HW_TAGS))                           \
> +               break;  /* No compiler instrumentation. */              \
> +       if (IS_ENABLED(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX))        \
> +               break;  /* Should always be instrumented! */            \
> +       if (IS_ENABLED(CONFIG_GENERIC_ENTRY))                           \
> +               kunit_skip((test), "Test requires checked mem*()");     \
> +} while (0)
> +
>  static void kmalloc_oob_right(struct kunit *test)
>  {
>         char *ptr;
> @@ -454,6 +463,8 @@ static void kmalloc_oob_16(struct kunit *test)
>                 u64 words[2];
>         } *ptr1, *ptr2;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         /* This test is specifically crafted for the generic mode. */
>         KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
>
> @@ -476,6 +487,8 @@ static void kmalloc_uaf_16(struct kunit *test)
>                 u64 words[2];
>         } *ptr1, *ptr2;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr1 = kmalloc(sizeof(*ptr1), GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr1);
>
> @@ -498,6 +511,8 @@ static void kmalloc_oob_memset_2(struct kunit *test)
>         char *ptr;
>         size_t size = 128 - KASAN_GRANULE_SIZE;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -511,6 +526,8 @@ static void kmalloc_oob_memset_4(struct kunit *test)
>         char *ptr;
>         size_t size = 128 - KASAN_GRANULE_SIZE;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -524,6 +541,8 @@ static void kmalloc_oob_memset_8(struct kunit *test)
>         char *ptr;
>         size_t size = 128 - KASAN_GRANULE_SIZE;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -537,6 +556,8 @@ static void kmalloc_oob_memset_16(struct kunit *test)
>         char *ptr;
>         size_t size = 128 - KASAN_GRANULE_SIZE;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -550,6 +571,8 @@ static void kmalloc_oob_in_memset(struct kunit *test)
>         char *ptr;
>         size_t size = 128 - KASAN_GRANULE_SIZE;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -566,6 +589,8 @@ static void kmalloc_memmove_negative_size(struct kunit *test)
>         size_t size = 64;
>         size_t invalid_size = -2;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         /*
>          * Hardware tag-based mode doesn't check memmove for negative size.
>          * As a result, this test introduces a side-effect memory corruption,
> @@ -590,6 +615,8 @@ static void kmalloc_memmove_invalid_size(struct kunit *test)
>         size_t size = 64;
>         size_t invalid_size = size;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> @@ -618,6 +645,8 @@ static void kmalloc_uaf_memset(struct kunit *test)
>         char *ptr;
>         size_t size = 33;
>
> +       KASAN_TEST_NEEDS_CHECKED_MEMINTRINSICS(test);
> +
>         /*
>          * Only generic KASAN uses quarantine, which is required to avoid a
>          * kernel memory corruption this test causes.
> --
> 2.39.2.637.g21b0678d19-goog
>

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

Thank you for taking care of all of this, Marco!

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ