From: Andrey Konovalov <[email protected]>
KASAN tests do out-of-bounds and use-after-free accesses. Running the
tests works fine for the GENERIC mode, as it uses qurantine and redzones.
But the HW_TAGS mode uses neither, and running the tests might crash
the kernel.
Rework the tests to avoid corrupting kernel memory.
Andrey Konovalov (8):
kasan: test: rework kmalloc_oob_right
kasan: test: avoid writing invalid memory
kasan: test: avoid corrupting memory via memset
kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS
kasan: test: only do kmalloc_uaf_memset for generic mode
kasan: test: clean up ksize_uaf
kasan: test: avoid corrupting memory in copy_user_test
kasan: test: avoid corrupting memory in kasan_rcu_uaf
lib/test_kasan.c | 74 ++++++++++++++++++++++++++++-------------
lib/test_kasan_module.c | 20 +++++------
2 files changed, 60 insertions(+), 34 deletions(-)
--
2.25.1
From: Andrey Konovalov <[email protected]>
kmalloc_oob_memset_*() tests do writes past the allocated objects.
As the result, they corrupt memory, which might lead to crashes with the
HW_TAGS mode, as it neither uses quarantine nor redzones.
Adjust the tests to only write memory within the aligned kmalloc objects.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index c82a82eb5393..fd00cd35e82c 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -431,61 +431,61 @@ static void kmalloc_uaf_16(struct kunit *test)
static void kmalloc_oob_memset_2(struct kunit *test)
{
char *ptr;
- size_t size = 8;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
+ KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));
kfree(ptr);
}
static void kmalloc_oob_memset_4(struct kunit *test)
{
char *ptr;
- size_t size = 8;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 5 + OOB_TAG_OFF, 0, 4));
+ KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 4));
kfree(ptr);
}
-
static void kmalloc_oob_memset_8(struct kunit *test)
{
char *ptr;
- size_t size = 8;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 8));
+ KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 8));
kfree(ptr);
}
static void kmalloc_oob_memset_16(struct kunit *test)
{
char *ptr;
- size_t size = 16;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 16));
+ KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 16));
kfree(ptr);
}
static void kmalloc_oob_in_memset(struct kunit *test)
{
char *ptr;
- size_t size = 666;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr, 0, size + 5 + OOB_TAG_OFF));
+ KUNIT_EXPECT_KASAN_FAIL(test,
+ memset(ptr, 0, size + KASAN_GRANULE_SIZE));
kfree(ptr);
}
--
2.25.1
From: Andrey Konovalov <[email protected]>
Multiple KASAN tests do writes past the allocated objects or writes to
freed memory. Turn these writes into reads to avoid corrupting memory.
Otherwise, these tests might lead to crashes with the HW_TAGS mode, as it
neither uses quarantine nor redzones.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 1bc3cdd2957f..c82a82eb5393 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -167,7 +167,7 @@ static void kmalloc_node_oob_right(struct kunit *test)
ptr = kmalloc_node(size, GFP_KERNEL, 0);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 0);
+ KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = ptr[size]);
kfree(ptr);
}
@@ -203,7 +203,7 @@ static void kmalloc_pagealloc_uaf(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
kfree(ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = 0);
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
}
static void kmalloc_pagealloc_invalid_free(struct kunit *test)
@@ -237,7 +237,7 @@ static void pagealloc_oob_right(struct kunit *test)
ptr = page_address(pages);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 0);
+ KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = ptr[size]);
free_pages((unsigned long)ptr, order);
}
@@ -252,7 +252,7 @@ static void pagealloc_uaf(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
free_pages((unsigned long)ptr, order);
- KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = 0);
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
}
static void kmalloc_large_oob_right(struct kunit *test)
@@ -514,7 +514,7 @@ static void kmalloc_uaf(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
kfree(ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, *(ptr + 8) = 'x');
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[8]);
}
static void kmalloc_uaf_memset(struct kunit *test)
@@ -553,7 +553,7 @@ static void kmalloc_uaf2(struct kunit *test)
goto again;
}
- KUNIT_EXPECT_KASAN_FAIL(test, ptr1[40] = 'x');
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr1)[40]);
KUNIT_EXPECT_PTR_NE(test, ptr1, ptr2);
kfree(ptr2);
@@ -700,7 +700,7 @@ static void ksize_unpoisons_memory(struct kunit *test)
ptr[size] = 'x';
/* This one must. */
- KUNIT_EXPECT_KASAN_FAIL(test, ptr[real_size] = 'y');
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[real_size]);
kfree(ptr);
}
--
2.25.1
From: Andrey Konovalov <[email protected]>
The HW_TAGS mode doesn't check memmove for negative size. As a result,
the kmalloc_memmove_invalid_size test corrupts memory, which can result
in a crash.
Disable this test with HW_TAGS KASAN.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index fd00cd35e82c..0b5698cd7d1d 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -495,11 +495,17 @@ static void kmalloc_memmove_invalid_size(struct kunit *test)
size_t size = 64;
volatile size_t invalid_size = -2;
+ /*
+ * Hardware tag-based mode doesn't check memmove for negative size.
+ * As a result, this test introduces a side-effect memory corruption,
+ * which can result in a crash.
+ */
+ KASAN_TEST_NEEDS_CONFIG_OFF(test, CONFIG_KASAN_HW_TAGS);
+
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
memset((char *)ptr, 0, 64);
-
KUNIT_EXPECT_KASAN_FAIL(test,
memmove((char *)ptr, (char *)ptr + 4, invalid_size));
kfree(ptr);
--
2.25.1
From: Andrey Konovalov <[email protected]>
kmalloc_uaf_memset() writes to freed memory, which is only safe with the
GENERIC mode (as it uses quarantine). For other modes, this test corrupts
kernel memory, which might result in a crash.
Only enable kmalloc_uaf_memset() for the GENERIC mode.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 0b5698cd7d1d..efd0da5c750f 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -528,6 +528,12 @@ static void kmalloc_uaf_memset(struct kunit *test)
char *ptr;
size_t size = 33;
+ /*
+ * Only generic KASAN uses quarantine, which is required to avoid a
+ * kernel memory corruption this test causes.
+ */
+ KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
+
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
--
2.25.1
From: Andrey Konovalov <[email protected]>
Some KASAN tests use global variables to store function returns values
so that the compiler doesn't optimize away these functions.
ksize_uaf() doesn't call any functions, so it doesn't need to use
kasan_int_result. Use volatile accesses instead, to be consistent with
other similar tests.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index efd0da5c750f..e159d24b3b49 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -731,8 +731,8 @@ static void ksize_uaf(struct kunit *test)
kfree(ptr);
KUNIT_EXPECT_KASAN_FAIL(test, ksize(ptr));
- KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = *ptr);
- KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = *(ptr + size));
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
+ KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[size]);
}
static void kasan_stack_oob(struct kunit *test)
--
2.25.1
From: Andrey Konovalov <[email protected]>
copy_user_test() does writes past the allocated object. As the result,
it corrupts kernel memory, which might lead to crashes with the HW_TAGS
mode, as it neither uses quarantine nor redzones.
(Technically, this test can't yet be enabled with the HW_TAGS mode, but
this will be implemented in the future.)
Adjust the test to only write memory within the aligned kmalloc object.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan_module.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
index f1017f345d6c..fa73b9df0be4 100644
--- a/lib/test_kasan_module.c
+++ b/lib/test_kasan_module.c
@@ -15,13 +15,11 @@
#include "../mm/kasan/kasan.h"
-#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE)
-
static noinline void __init copy_user_test(void)
{
char *kmem;
char __user *usermem;
- size_t size = 10;
+ size_t size = 128 - KASAN_GRANULE_SIZE;
int __maybe_unused unused;
kmem = kmalloc(size, GFP_KERNEL);
@@ -38,25 +36,25 @@ static noinline void __init copy_user_test(void)
}
pr_info("out-of-bounds in copy_from_user()\n");
- unused = copy_from_user(kmem, usermem, size + 1 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ 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 + OOB_TAG_OFF);
+ unused = strncpy_from_user(kmem, usermem, size + 1);
vm_munmap((unsigned long)usermem, PAGE_SIZE);
kfree(kmem);
--
2.25.1
From: Andrey Konovalov <[email protected]>
kasan_rcu_uaf() writes to freed memory via kasan_rcu_reclaim(), which is
only safe with the GENERIC mode (as it uses quarantine). For other modes,
this test corrupts kernel memory, which might result in a crash.
Turn the write into a read.
Signed-off-by: Andrey Konovalov <[email protected]>
---
lib/test_kasan_module.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
index fa73b9df0be4..7ebf433edef3 100644
--- a/lib/test_kasan_module.c
+++ b/lib/test_kasan_module.c
@@ -71,7 +71,7 @@ static noinline void __init kasan_rcu_reclaim(struct rcu_head *rp)
struct kasan_rcu_info, rcu);
kfree(fp);
- fp->i = 1;
+ ((volatile struct kasan_rcu_info *)fp)->i;
}
static noinline void __init kasan_rcu_uaf(void)
--
2.25.1
On Wed, 11 Aug 2021 at 21:30, <[email protected]> wrote:
> From: Andrey Konovalov <[email protected]>
>
> copy_user_test() does writes past the allocated object. As the result,
> it corrupts kernel memory, which might lead to crashes with the HW_TAGS
> mode, as it neither uses quarantine nor redzones.
>
> (Technically, this test can't yet be enabled with the HW_TAGS mode, but
> this will be implemented in the future.)
>
> Adjust the test to only write memory within the aligned kmalloc object.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
> ---
> lib/test_kasan_module.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
> index f1017f345d6c..fa73b9df0be4 100644
> --- a/lib/test_kasan_module.c
> +++ b/lib/test_kasan_module.c
> @@ -15,13 +15,11 @@
>
> #include "../mm/kasan/kasan.h"
>
> -#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 : KASAN_GRANULE_SIZE)
> -
> static noinline void __init copy_user_test(void)
> {
> char *kmem;
> char __user *usermem;
> - size_t size = 10;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
> int __maybe_unused unused;
>
> kmem = kmalloc(size, GFP_KERNEL);
> @@ -38,25 +36,25 @@ static noinline void __init copy_user_test(void)
> }
>
> pr_info("out-of-bounds in copy_from_user()\n");
> - unused = copy_from_user(kmem, usermem, size + 1 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + 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 + OOB_TAG_OFF);
> + unused = strncpy_from_user(kmem, usermem, size + 1);
>
> vm_munmap((unsigned long)usermem, PAGE_SIZE);
> kfree(kmem);
> --
> 2.25.1
>
On Wed, 11 Aug 2021 at 21:34, <[email protected]> wrote:
>
> From: Andrey Konovalov <[email protected]>
>
> kasan_rcu_uaf() writes to freed memory via kasan_rcu_reclaim(), which is
> only safe with the GENERIC mode (as it uses quarantine). For other modes,
> this test corrupts kernel memory, which might result in a crash.
>
> Turn the write into a read.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
> ---
> lib/test_kasan_module.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
> index fa73b9df0be4..7ebf433edef3 100644
> --- a/lib/test_kasan_module.c
> +++ b/lib/test_kasan_module.c
> @@ -71,7 +71,7 @@ static noinline void __init kasan_rcu_reclaim(struct rcu_head *rp)
> struct kasan_rcu_info, rcu);
>
> kfree(fp);
> - fp->i = 1;
> + ((volatile struct kasan_rcu_info *)fp)->i;
> }
>
> static noinline void __init kasan_rcu_uaf(void)
> --
> 2.25.1
>
On Wed, 11 Aug 2021 at 21:23, <[email protected]> wrote:
>
> From: Andrey Konovalov <[email protected]>
>
> Some KASAN tests use global variables to store function returns values
> so that the compiler doesn't optimize away these functions.
>
> ksize_uaf() doesn't call any functions, so it doesn't need to use
> kasan_int_result. Use volatile accesses instead, to be consistent with
> other similar tests.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
Although I do wonder if the compiler might one day mess with the
volatile reads. At least this way we might also catch if the compiler
messes up volatile reads. ;-)
> ---
> lib/test_kasan.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index efd0da5c750f..e159d24b3b49 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -731,8 +731,8 @@ static void ksize_uaf(struct kunit *test)
> kfree(ptr);
>
> KUNIT_EXPECT_KASAN_FAIL(test, ksize(ptr));
> - KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = *ptr);
> - KUNIT_EXPECT_KASAN_FAIL(test, kasan_int_result = *(ptr + size));
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[size]);
> }
>
> static void kasan_stack_oob(struct kunit *test)
> --
> 2.25.1
>
On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> From: Andrey Konovalov <[email protected]>
>
> kmalloc_oob_memset_*() tests do writes past the allocated objects.
> As the result, they corrupt memory, which might lead to crashes with the
> HW_TAGS mode, as it neither uses quarantine nor redzones.
>
> Adjust the tests to only write memory within the aligned kmalloc objects.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
> ---
> lib/test_kasan.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index c82a82eb5393..fd00cd35e82c 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -431,61 +431,61 @@ static void kmalloc_uaf_16(struct kunit *test)
> static void kmalloc_oob_memset_2(struct kunit *test)
> {
> char *ptr;
> - size_t size = 8;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
>
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
> + KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));
I think one important aspect of these tests in generic mode is that
the written range touches both valid and invalid memory. I think that
was meant to test any explicit instrumentation isn't just looking at
the starting address, but at the whole range.
It seems that with these changes that is no longer tested. Could we
somehow make it still test that?
> kfree(ptr);
> }
>
> static void kmalloc_oob_memset_4(struct kunit *test)
> {
> char *ptr;
> - size_t size = 8;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
>
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 5 + OOB_TAG_OFF, 0, 4));
> + KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 4));
> kfree(ptr);
> }
>
> -
> static void kmalloc_oob_memset_8(struct kunit *test)
> {
> char *ptr;
> - size_t size = 8;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
>
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 8));
> + KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 8));
> kfree(ptr);
> }
>
> static void kmalloc_oob_memset_16(struct kunit *test)
> {
> char *ptr;
> - size_t size = 16;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
>
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 1 + OOB_TAG_OFF, 0, 16));
> + KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 16));
> kfree(ptr);
> }
>
> static void kmalloc_oob_in_memset(struct kunit *test)
> {
> char *ptr;
> - size_t size = 666;
> + size_t size = 128 - KASAN_GRANULE_SIZE;
>
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr, 0, size + 5 + OOB_TAG_OFF));
> + KUNIT_EXPECT_KASAN_FAIL(test,
> + memset(ptr, 0, size + KASAN_GRANULE_SIZE));
> kfree(ptr);
> }
>
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/e9e2f7180f96e2496f0249ac81887376c6171e8f.1628709663.git.andreyknvl%40gmail.com.
On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> From: Andrey Konovalov <[email protected]>
>
> Multiple KASAN tests do writes past the allocated objects or writes to
> freed memory. Turn these writes into reads to avoid corrupting memory.
> Otherwise, these tests might lead to crashes with the HW_TAGS mode, as it
> neither uses quarantine nor redzones.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
although if you need a write primitive somewhere that doesn't corrupt
memory, you could use atomic_add() or atomic_or() of 0. Although
technically that's a read-modify-write. For generic mode one issue is
that these are explicitly instrumented and not through the compiler,
which is only a problem if you're testing the compiler emits the right
instrumentation.
> ---
> lib/test_kasan.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 1bc3cdd2957f..c82a82eb5393 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -167,7 +167,7 @@ static void kmalloc_node_oob_right(struct kunit *test)
> ptr = kmalloc_node(size, GFP_KERNEL, 0);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 0);
> + KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = ptr[size]);
> kfree(ptr);
> }
>
> @@ -203,7 +203,7 @@ static void kmalloc_pagealloc_uaf(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> kfree(ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = 0);
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
> }
>
> static void kmalloc_pagealloc_invalid_free(struct kunit *test)
> @@ -237,7 +237,7 @@ static void pagealloc_oob_right(struct kunit *test)
> ptr = page_address(pages);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 0);
> + KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = ptr[size]);
> free_pages((unsigned long)ptr, order);
> }
>
> @@ -252,7 +252,7 @@ static void pagealloc_uaf(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> free_pages((unsigned long)ptr, order);
>
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] = 0);
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[0]);
> }
>
> static void kmalloc_large_oob_right(struct kunit *test)
> @@ -514,7 +514,7 @@ static void kmalloc_uaf(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> kfree(ptr);
> - KUNIT_EXPECT_KASAN_FAIL(test, *(ptr + 8) = 'x');
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[8]);
> }
>
> static void kmalloc_uaf_memset(struct kunit *test)
> @@ -553,7 +553,7 @@ static void kmalloc_uaf2(struct kunit *test)
> goto again;
> }
>
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr1[40] = 'x');
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr1)[40]);
> KUNIT_EXPECT_PTR_NE(test, ptr1, ptr2);
>
> kfree(ptr2);
> @@ -700,7 +700,7 @@ static void ksize_unpoisons_memory(struct kunit *test)
> ptr[size] = 'x';
>
> /* This one must. */
> - KUNIT_EXPECT_KASAN_FAIL(test, ptr[real_size] = 'y');
> + KUNIT_EXPECT_KASAN_FAIL(test, ((volatile char *)ptr)[real_size]);
>
> kfree(ptr);
> }
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/c3cd2a383e757e27dd9131635fc7d09a48a49cf9.1628709663.git.andreyknvl%40gmail.com.
On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> From: Andrey Konovalov <[email protected]>
>
> kmalloc_uaf_memset() writes to freed memory, which is only safe with the
> GENERIC mode (as it uses quarantine). For other modes, this test corrupts
> kernel memory, which might result in a crash.
>
> Only enable kmalloc_uaf_memset() for the GENERIC mode.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Acked-by: Marco Elver <[email protected]>
> ---
> lib/test_kasan.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 0b5698cd7d1d..efd0da5c750f 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -528,6 +528,12 @@ static void kmalloc_uaf_memset(struct kunit *test)
> char *ptr;
> size_t size = 33;
>
> + /*
> + * Only generic KASAN uses quarantine, which is required to avoid a
> + * kernel memory corruption this test causes.
> + */
> + KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
> +
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> --
> 2.25.1
On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> From: Andrey Konovalov <[email protected]>
>
> The HW_TAGS mode doesn't check memmove for negative size. As a result,
> the kmalloc_memmove_invalid_size test corrupts memory, which can result
> in a crash.
>
> Disable this test with HW_TAGS KASAN.
>
> Signed-off-by: Andrey Konovalov <[email protected]>
Reviewed-by: Marco Elver <[email protected]>
> ---
> lib/test_kasan.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index fd00cd35e82c..0b5698cd7d1d 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -495,11 +495,17 @@ static void kmalloc_memmove_invalid_size(struct kunit *test)
> size_t size = 64;
> volatile size_t invalid_size = -2;
>
> + /*
> + * Hardware tag-based mode doesn't check memmove for negative size.
> + * As a result, this test introduces a side-effect memory corruption,
> + * which can result in a crash.
> + */
> + KASAN_TEST_NEEDS_CONFIG_OFF(test, CONFIG_KASAN_HW_TAGS);
> +
> ptr = kmalloc(size, GFP_KERNEL);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> memset((char *)ptr, 0, 64);
> -
> KUNIT_EXPECT_KASAN_FAIL(test,
> memmove((char *)ptr, (char *)ptr + 4, invalid_size));
> kfree(ptr);
> --
> 2.25.1
>
> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/408c63e4a0353633a13403aab4ff25a505e03d93.1628709663.git.andreyknvl%40gmail.com.
On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
>
> From: Andrey Konovalov <[email protected]>
>
> KASAN tests do out-of-bounds and use-after-free accesses. Running the
> tests works fine for the GENERIC mode, as it uses qurantine and redzones.
> But the HW_TAGS mode uses neither, and running the tests might crash
> the kernel.
>
> Rework the tests to avoid corrupting kernel memory.
Thanks for this!
I think only 1 change is questionable ("kasan: test: avoid corrupting
memory via memset") because it no longer checks overlapping valid to
invalid range writes.
> Andrey Konovalov (8):
> kasan: test: rework kmalloc_oob_right
> kasan: test: avoid writing invalid memory
> kasan: test: avoid corrupting memory via memset
> kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS
> kasan: test: only do kmalloc_uaf_memset for generic mode
> kasan: test: clean up ksize_uaf
> kasan: test: avoid corrupting memory in copy_user_test
> kasan: test: avoid corrupting memory in kasan_rcu_uaf
>
> lib/test_kasan.c | 74 ++++++++++++++++++++++++++++-------------
> lib/test_kasan_module.c | 20 +++++------
> 2 files changed, 60 insertions(+), 34 deletions(-)
>
> --
> 2.25.1
>
On Thu, Aug 12, 2021 at 10:57 AM Marco Elver <[email protected]> wrote:
>
> On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> > From: Andrey Konovalov <[email protected]>
> >
> > kmalloc_oob_memset_*() tests do writes past the allocated objects.
> > As the result, they corrupt memory, which might lead to crashes with the
> > HW_TAGS mode, as it neither uses quarantine nor redzones.
> >
> > Adjust the tests to only write memory within the aligned kmalloc objects.
> >
> > Signed-off-by: Andrey Konovalov <[email protected]>
> > ---
> > lib/test_kasan.c | 22 +++++++++++-----------
> > 1 file changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> > index c82a82eb5393..fd00cd35e82c 100644
> > --- a/lib/test_kasan.c
> > +++ b/lib/test_kasan.c
> > @@ -431,61 +431,61 @@ static void kmalloc_uaf_16(struct kunit *test)
> > static void kmalloc_oob_memset_2(struct kunit *test)
> > {
> > char *ptr;
> > - size_t size = 8;
> > + size_t size = 128 - KASAN_GRANULE_SIZE;
> >
> > ptr = kmalloc(size, GFP_KERNEL);
> > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
> >
> > - KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + 7 + OOB_TAG_OFF, 0, 2));
> > + KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size, 0, 2));
>
> I think one important aspect of these tests in generic mode is that
> the written range touches both valid and invalid memory. I think that
> was meant to test any explicit instrumentation isn't just looking at
> the starting address, but at the whole range.
Good point!
> It seems that with these changes that is no longer tested. Could we
> somehow make it still test that?
Yes, will do in v2.
Thanks, Marco!
On Thu, Aug 12, 2021 at 10:57 AM Marco Elver <[email protected]> wrote:
>
> On Wed, 11 Aug 2021 at 21:21, <[email protected]> wrote:
> > From: Andrey Konovalov <[email protected]>
> >
> > Multiple KASAN tests do writes past the allocated objects or writes to
> > freed memory. Turn these writes into reads to avoid corrupting memory.
> > Otherwise, these tests might lead to crashes with the HW_TAGS mode, as it
> > neither uses quarantine nor redzones.
> >
> > Signed-off-by: Andrey Konovalov <[email protected]>
>
> Reviewed-by: Marco Elver <[email protected]>
>
> although if you need a write primitive somewhere that doesn't corrupt
> memory, you could use atomic_add() or atomic_or() of 0. Although
> technically that's a read-modify-write.
Interesting idea. I'd say let's keep the volatile reads for now, and
change them if we encounter any problem with those.
> For generic mode one issue is
> that these are explicitly instrumented and not through the compiler,
> which is only a problem if you're testing the compiler emits the right
> instrumentation.
On a related point, it seems we have no KASAN tests to check atomic operations.
Filed https://bugzilla.kernel.org/show_bug.cgi?id=214055 for this.
Thanks!