On Fri, Feb 17, 2023 at 12:45 AM Marco Elver <elver@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> ---
> 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@xxxxxxxxx>
Thank you for taking care of all of this, Marco!
