Re: [PATCH] kasan: migrate copy_user_test to kunit
From: Sabyrzhan Tasbolatov
Date: Fri Oct 11 2024 - 05:12:25 EST
On Fri, Oct 11, 2024 at 12:16 PM Sabyrzhan Tasbolatov
<snovitoll@xxxxxxxxx> wrote:
>
> Migrate the copy_user_test to the KUnit framework to verify out-of-bound
> detection via KASAN reports in copy_from_user(), copy_to_user() and
> their static functions.
>
> This is the last migrated test in kasan_test_module.c, therefore delete
> the file.
>
> In order to detect OOB access in strncpy_from_user(), we need to move
> kasan_check_write() to the function beginning to cover
> if (can_do_masked_user_access()) {...} branch as well.
>
> Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxx>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=212205
> Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@xxxxxxxxx>
> ---
> lib/strncpy_from_user.c | 3 +-
> mm/kasan/kasan_test_c.c | 39 +++++++++++++++++
> mm/kasan/kasan_test_module.c | 81 ------------------------------------
> 3 files changed, 41 insertions(+), 82 deletions(-)
> delete mode 100644 mm/kasan/kasan_test_module.c
>
> diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
> index 989a12a67872..55c33e4f3c70 100644
> --- a/lib/strncpy_from_user.c
> +++ b/lib/strncpy_from_user.c
> @@ -120,6 +120,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
> if (unlikely(count <= 0))
> return 0;
>
> + kasan_check_write(dst, count);
> +
> if (can_do_masked_user_access()) {
> long retval;
>
> @@ -142,7 +144,6 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
> if (max > count)
> max = count;
>
> - kasan_check_write(dst, count);
> check_object_size(dst, count, false);
> if (user_read_access_begin(src, max)) {
> retval = do_strncpy_from_user(dst, src, count, max);
> diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
> index a181e4780d9d..e71a16d0dfb9 100644
> --- a/mm/kasan/kasan_test_c.c
> +++ b/mm/kasan/kasan_test_c.c
> @@ -1954,6 +1954,44 @@ static void rust_uaf(struct kunit *test)
> KUNIT_EXPECT_KASAN_FAIL(test, kasan_test_rust_uaf());
> }
>
> +static void copy_user_test_oob(struct kunit *test)
> +{
> + char *kmem;
> + char __user *usermem;
> + unsigned long useraddr;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
> + int __maybe_unused unused;
> +
> + kmem = kunit_kmalloc(test, size, GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, kmem);
> +
> + useraddr = kunit_vm_mmap(test, NULL, 0, PAGE_SIZE,
> + PROT_READ | PROT_WRITE | PROT_EXEC,
> + MAP_ANONYMOUS | MAP_PRIVATE, 0);
> + KUNIT_ASSERT_NE_MSG(test, useraddr, 0,
> + "Could not create userspace mm");
> + KUNIT_ASSERT_LT_MSG(test, useraddr, (unsigned long)TASK_SIZE,
> + "Failed to allocate user memory");
> +
> + OPTIMIZER_HIDE_VAR(size);
> + usermem = (char __user *)useraddr;
> +
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = copy_from_user(kmem, usermem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = copy_to_user(usermem, kmem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = __copy_from_user(kmem, usermem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = __copy_to_user(usermem, kmem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = __copy_from_user_inatomic(kmem, usermem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = __copy_to_user_inatomic(usermem, kmem, size + 1));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + unused = strncpy_from_user(kmem, usermem, size + 1));
> +}
> +
> static struct kunit_case kasan_kunit_test_cases[] = {
> KUNIT_CASE(kmalloc_oob_right),
> KUNIT_CASE(kmalloc_oob_left),
> @@ -2028,6 +2066,7 @@ static struct kunit_case kasan_kunit_test_cases[] = {
> KUNIT_CASE(match_all_ptr_tag),
> KUNIT_CASE(match_all_mem_tag),
> KUNIT_CASE(rust_uaf),
> + KUNIT_CASE(copy_user_test_oob),
> {}
> };
>
> diff --git a/mm/kasan/kasan_test_module.c b/mm/kasan/kasan_test_module.c
> deleted file mode 100644
> index 27ec22767e42..000000000000
> --- a/mm/kasan/kasan_test_module.c
> +++ /dev/null
> @@ -1,81 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - *
> - * Copyright (c) 2014 Samsung Electronics Co., Ltd.
> - * Author: Andrey Ryabinin <a.ryabinin@xxxxxxxxxxx>
> - */
> -
> -#define pr_fmt(fmt) "kasan: test: " fmt
> -
> -#include <linux/mman.h>
> -#include <linux/module.h>
> -#include <linux/printk.h>
> -#include <linux/slab.h>
> -#include <linux/uaccess.h>
> -
> -#include "kasan.h"
> -
> -static noinline void __init copy_user_test(void)
> -{
> - char *kmem;
> - char __user *usermem;
> - size_t size = 128 - KASAN_GRANULE_SIZE;
> - int __maybe_unused unused;
> -
> - kmem = kmalloc(size, GFP_KERNEL);
> - if (!kmem)
> - return;
> -
> - usermem = (char __user *)vm_mmap(NULL, 0, PAGE_SIZE,
> - PROT_READ | PROT_WRITE | PROT_EXEC,
> - MAP_ANONYMOUS | MAP_PRIVATE, 0);
> - if (IS_ERR(usermem)) {
> - pr_err("Failed to allocate user memory\n");
> - kfree(kmem);
> - return;
> - }
> -
> - OPTIMIZER_HIDE_VAR(size);
> -
> - pr_info("out-of-bounds in copy_from_user()\n");
> - unused = copy_from_user(kmem, usermem, size + 1);
> -
> - pr_info("out-of-bounds in copy_to_user()\n");
> - unused = copy_to_user(usermem, kmem, size + 1);
> -
> - pr_info("out-of-bounds in __copy_from_user()\n");
> - unused = __copy_from_user(kmem, usermem, size + 1);
> -
> - pr_info("out-of-bounds in __copy_to_user()\n");
> - unused = __copy_to_user(usermem, kmem, size + 1);
> -
> - pr_info("out-of-bounds in __copy_from_user_inatomic()\n");
> - unused = __copy_from_user_inatomic(kmem, usermem, size + 1);
> -
> - pr_info("out-of-bounds in __copy_to_user_inatomic()\n");
> - unused = __copy_to_user_inatomic(usermem, kmem, size + 1);
> -
> - pr_info("out-of-bounds in strncpy_from_user()\n");
> - unused = strncpy_from_user(kmem, usermem, size + 1);
> -
> - vm_munmap((unsigned long)usermem, PAGE_SIZE);
> - kfree(kmem);
> -}
> -
> -static int __init kasan_test_module_init(void)
> -{
> - /*
> - * Temporarily enable multi-shot mode. Otherwise, KASAN would only
> - * report the first detected bug and panic the kernel if panic_on_warn
> - * is enabled.
> - */
> - bool multishot = kasan_save_enable_multi_shot();
> -
> - copy_user_test();
> -
> - kasan_restore_multi_shot(multishot);
> - return -EAGAIN;
> -}
> -
> -module_init(kasan_test_module_init);
> -MODULE_LICENSE("GPL");
> --
> 2.34.1
>
This has been tested on:
- x86_64 with CONFIG_KASAN_GENERIC
- arm64 with CONFIG_KASAN_SW_TAGS
- arm64 with CONFIG_KASAN_HW_TAGS
- arm64 SW_TAGS has 1 failing test which is in the mainline,
will try to address it in different patch, not related to changes in this PR:
[ 9.480716] # vmalloc_percpu: EXPECTATION FAILED at
mm/kasan/kasan_test_c.c:1830
[ 9.480716] Expected (u8)(__u8)((u64)(c_ptr) >> 56) < (u8)0xFF, but
[ 9.480716] (u8)(__u8)((u64)(c_ptr) >> 56) == 255 (0xff)
[ 9.480716] (u8)0xFF == 255 (0xff)
[ 9.481936] # vmalloc_percpu: EXPECTATION FAILED at
mm/kasan/kasan_test_c.c:1830
[ 9.481936] Expected (u8)(__u8)((u64)(c_ptr) >> 56) < (u8)0xFF, but
[ 9.481936] (u8)(__u8)((u64)(c_ptr) >> 56) == 255 (0xff)
[ 9.481936] (u8)0xFF == 255 (0xff)
Here is my full console log of arm64-sw.log:
https://gist.githubusercontent.com/novitoll/7ab93edca1f7d71925735075e84fc2ec/raw/6ef05758bcc396cd2f5796a5bcb5e41a091224cf/arm64-sw.log
- arm64 HW_TAGS has 1 failing test related to new changes
and AFAIU, it's known issue related to HW_TAGS:
[ 11.167324] # copy_user_test_oob: EXPECTATION FAILED at
mm/kasan/kasan_test_c.c:1992
[ 11.167324] KASAN failure expected in "unused =
strncpy_from_user(kmem, usermem, size + 1)", but none occurred
Here is the console log of arm64-hw.log:
https://gist.github.com/novitoll/7ab93edca1f7d71925735075e84fc2ec#file-arm64-hw-log-L11208