linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS
@ 2021-08-11 19:21 andrey.konovalov
  2021-08-11 19:21 ` [PATCH 1/8] kasan: test: rework kmalloc_oob_right andrey.konovalov
                   ` (8 more replies)
  0 siblings, 9 replies; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/8] kasan: test: rework kmalloc_oob_right
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
@ 2021-08-11 19:21 ` andrey.konovalov
  2021-08-12  8:57   ` Marco Elver
  2021-08-11 19:21 ` [PATCH 2/8] kasan: test: avoid writing invalid memory andrey.konovalov
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

Rework kmalloc_oob_right() to do these bad access checks:

1. An unaligned access one byte past the requested kmalloc size
   (can only be detected by KASAN_GENERIC).
2. An aligned access into the first out-of-bounds granule that falls
   within the aligned kmalloc object.
3. Out-of-bounds access past the aligned kmalloc object.

Test #3 deliberately uses a read access to avoid corrupting memory.
Otherwise, this test might lead to crashes with the HW_TAGS mode, as it
neither uses quarantine nor redzones.

Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>
---
 lib/test_kasan.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 8f7b0b2f6e11..1bc3cdd2957f 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -122,12 +122,28 @@ static void kasan_test_exit(struct kunit *test)
 static void kmalloc_oob_right(struct kunit *test)
 {
 	char *ptr;
-	size_t size = 123;
+	size_t size = 128 - KASAN_GRANULE_SIZE - 5;
 
 	ptr = kmalloc(size, GFP_KERNEL);
 	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
 
-	KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + OOB_TAG_OFF] = 'x');
+	/*
+	 * An unaligned access past the requested kmalloc size.
+	 * Only generic KASAN can precisely detect these.
+	 */
+	if (IS_ENABLED(CONFIG_KASAN_GENERIC))
+		KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'x');
+
+	/*
+	 * An aligned access into the first out-of-bounds granule that falls
+	 * within the aligned kmalloc object.
+	 */
+	KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + 5] = 'y');
+
+	/* Out-of-bounds access past the aligned kmalloc object. */
+	KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] =
+					ptr[size + KASAN_GRANULE_SIZE + 5]);
+
 	kfree(ptr);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/8] kasan: test: avoid writing invalid memory
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
  2021-08-11 19:21 ` [PATCH 1/8] kasan: test: rework kmalloc_oob_right andrey.konovalov
@ 2021-08-11 19:21 ` andrey.konovalov
  2021-08-12  8:57   ` Marco Elver
  2021-08-11 19:21 ` [PATCH 3/8] kasan: test: avoid corrupting memory via memset andrey.konovalov
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/8] kasan: test: avoid corrupting memory via memset
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
  2021-08-11 19:21 ` [PATCH 1/8] kasan: test: rework kmalloc_oob_right andrey.konovalov
  2021-08-11 19:21 ` [PATCH 2/8] kasan: test: avoid writing invalid memory andrey.konovalov
@ 2021-08-11 19:21 ` andrey.konovalov
  2021-08-12  8:56   ` Marco Elver
  2021-08-11 19:21 ` [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS andrey.konovalov
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (2 preceding siblings ...)
  2021-08-11 19:21 ` [PATCH 3/8] kasan: test: avoid corrupting memory via memset andrey.konovalov
@ 2021-08-11 19:21 ` andrey.konovalov
  2021-08-12  8:57   ` Marco Elver
  2021-08-11 19:21 ` [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode andrey.konovalov
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (3 preceding siblings ...)
  2021-08-11 19:21 ` [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS andrey.konovalov
@ 2021-08-11 19:21 ` andrey.konovalov
  2021-08-12  8:56   ` Marco Elver
  2021-08-11 19:23 ` [PATCH 6/8] kasan: test: clean up ksize_uaf andrey.konovalov
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/8] kasan: test: clean up ksize_uaf
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (4 preceding siblings ...)
  2021-08-11 19:21 ` [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode andrey.konovalov
@ 2021-08-11 19:23 ` andrey.konovalov
  2021-08-12  8:56   ` Marco Elver
  2021-08-11 19:30 ` [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test andrey.konovalov
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (5 preceding siblings ...)
  2021-08-11 19:23 ` [PATCH 6/8] kasan: test: clean up ksize_uaf andrey.konovalov
@ 2021-08-11 19:30 ` andrey.konovalov
  2021-08-12  8:50   ` Marco Elver
  2021-08-11 19:34 ` [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf andrey.konovalov
  2021-08-12  8:58 ` [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS Marco Elver
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (6 preceding siblings ...)
  2021-08-11 19:30 ` [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test andrey.konovalov
@ 2021-08-11 19:34 ` andrey.konovalov
  2021-08-12  8:50   ` Marco Elver
  2021-08-12  8:58 ` [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS Marco Elver
  8 siblings, 1 reply; 20+ messages in thread
From: andrey.konovalov @ 2021-08-11 19:34 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Andrey Konovalov, Andrey Ryabinin, Marco Elver, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

From: Andrey Konovalov <andreyknvl@gmail.com>

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 <andreyknvl@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf
  2021-08-11 19:34 ` [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf andrey.konovalov
@ 2021-08-12  8:50   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:50 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:34, <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>


> ---
>  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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test
  2021-08-11 19:30 ` [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test andrey.konovalov
@ 2021-08-12  8:50   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:50 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:30, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>



> ---
>  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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/8] kasan: test: clean up ksize_uaf
  2021-08-11 19:23 ` [PATCH 6/8] kasan: test: clean up ksize_uaf andrey.konovalov
@ 2021-08-12  8:56   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:56 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:23, <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>

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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode
  2021-08-11 19:21 ` [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode andrey.konovalov
@ 2021-08-12  8:56   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:56 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Acked-by: Marco Elver <elver@google.com>


> ---
>  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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/8] kasan: test: avoid corrupting memory via memset
  2021-08-11 19:21 ` [PATCH 3/8] kasan: test: avoid corrupting memory via memset andrey.konovalov
@ 2021-08-12  8:56   ` Marco Elver
  2021-08-12 12:55     ` Andrey Konovalov
  0 siblings, 1 reply; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:56 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>
> ---
>  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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/e9e2f7180f96e2496f0249ac81887376c6171e8f.1628709663.git.andreyknvl%40gmail.com.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/8] kasan: test: avoid writing invalid memory
  2021-08-11 19:21 ` [PATCH 2/8] kasan: test: avoid writing invalid memory andrey.konovalov
@ 2021-08-12  8:57   ` Marco Elver
  2021-08-12 13:02     ` Andrey Konovalov
  0 siblings, 1 reply; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:57 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>

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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/c3cd2a383e757e27dd9131635fc7d09a48a49cf9.1628709663.git.andreyknvl%40gmail.com.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/8] kasan: test: rework kmalloc_oob_right
  2021-08-11 19:21 ` [PATCH 1/8] kasan: test: rework kmalloc_oob_right andrey.konovalov
@ 2021-08-12  8:57   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:57 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> Rework kmalloc_oob_right() to do these bad access checks:
>
> 1. An unaligned access one byte past the requested kmalloc size
>    (can only be detected by KASAN_GENERIC).
> 2. An aligned access into the first out-of-bounds granule that falls
>    within the aligned kmalloc object.
> 3. Out-of-bounds access past the aligned kmalloc object.
>
> Test #3 deliberately uses a read access to avoid corrupting memory.
> Otherwise, this test might lead to crashes with the HW_TAGS mode, as it
> neither uses quarantine nor redzones.
>
> Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>


> ---
>  lib/test_kasan.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 8f7b0b2f6e11..1bc3cdd2957f 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -122,12 +122,28 @@ static void kasan_test_exit(struct kunit *test)
>  static void kmalloc_oob_right(struct kunit *test)
>  {
>         char *ptr;
> -       size_t size = 123;
> +       size_t size = 128 - KASAN_GRANULE_SIZE - 5;
>
>         ptr = kmalloc(size, GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> -       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + OOB_TAG_OFF] = 'x');
> +       /*
> +        * An unaligned access past the requested kmalloc size.
> +        * Only generic KASAN can precisely detect these.
> +        */
> +       if (IS_ENABLED(CONFIG_KASAN_GENERIC))
> +               KUNIT_EXPECT_KASAN_FAIL(test, ptr[size] = 'x');
> +
> +       /*
> +        * An aligned access into the first out-of-bounds granule that falls
> +        * within the aligned kmalloc object.
> +        */
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + 5] = 'y');
> +
> +       /* Out-of-bounds access past the aligned kmalloc object. */
> +       KUNIT_EXPECT_KASAN_FAIL(test, ptr[0] =
> +                                       ptr[size + KASAN_GRANULE_SIZE + 5]);
> +
>         kfree(ptr);
>  }
>
> --
> 2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS
  2021-08-11 19:21 ` [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS andrey.konovalov
@ 2021-08-12  8:57   ` Marco Elver
  0 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:57 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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 <andreyknvl@gmail.com>

Reviewed-by: Marco Elver <elver@google.com>



> ---
>  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 kasan-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/408c63e4a0353633a13403aab4ff25a505e03d93.1628709663.git.andreyknvl%40gmail.com.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS
  2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
                   ` (7 preceding siblings ...)
  2021-08-11 19:34 ` [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf andrey.konovalov
@ 2021-08-12  8:58 ` Marco Elver
  8 siblings, 0 replies; 20+ messages in thread
From: Marco Elver @ 2021-08-12  8:58 UTC (permalink / raw)
  To: andrey.konovalov
  Cc: Andrew Morton, Andrey Konovalov, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, linux-mm, linux-kernel

On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@gmail.com>
>
> 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
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/8] kasan: test: avoid corrupting memory via memset
  2021-08-12  8:56   ` Marco Elver
@ 2021-08-12 12:55     ` Andrey Konovalov
  0 siblings, 0 replies; 20+ messages in thread
From: Andrey Konovalov @ 2021-08-12 12:55 UTC (permalink / raw)
  To: Marco Elver
  Cc: andrey.konovalov, Andrew Morton, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, Linux Memory Management List,
	LKML

On Thu, Aug 12, 2021 at 10:57 AM Marco Elver <elver@google.com> wrote:
>
> On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> > From: Andrey Konovalov <andreyknvl@gmail.com>
> >
> > 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 <andreyknvl@gmail.com>
> > ---
> >  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!

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/8] kasan: test: avoid writing invalid memory
  2021-08-12  8:57   ` Marco Elver
@ 2021-08-12 13:02     ` Andrey Konovalov
  0 siblings, 0 replies; 20+ messages in thread
From: Andrey Konovalov @ 2021-08-12 13:02 UTC (permalink / raw)
  To: Marco Elver
  Cc: andrey.konovalov, Andrew Morton, Andrey Ryabinin, Dmitry Vyukov,
	Alexander Potapenko, kasan-dev, Linux Memory Management List,
	LKML

On Thu, Aug 12, 2021 at 10:57 AM Marco Elver <elver@google.com> wrote:
>
> On Wed, 11 Aug 2021 at 21:21, <andrey.konovalov@linux.dev> wrote:
> > From: Andrey Konovalov <andreyknvl@gmail.com>
> >
> > 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 <andreyknvl@gmail.com>
>
> Reviewed-by: Marco Elver <elver@google.com>
>
> 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!

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-08-12 13:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 19:21 [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS andrey.konovalov
2021-08-11 19:21 ` [PATCH 1/8] kasan: test: rework kmalloc_oob_right andrey.konovalov
2021-08-12  8:57   ` Marco Elver
2021-08-11 19:21 ` [PATCH 2/8] kasan: test: avoid writing invalid memory andrey.konovalov
2021-08-12  8:57   ` Marco Elver
2021-08-12 13:02     ` Andrey Konovalov
2021-08-11 19:21 ` [PATCH 3/8] kasan: test: avoid corrupting memory via memset andrey.konovalov
2021-08-12  8:56   ` Marco Elver
2021-08-12 12:55     ` Andrey Konovalov
2021-08-11 19:21 ` [PATCH 4/8] kasan: test: disable kmalloc_memmove_invalid_size for HW_TAGS andrey.konovalov
2021-08-12  8:57   ` Marco Elver
2021-08-11 19:21 ` [PATCH 5/8] kasan: test: only do kmalloc_uaf_memset for generic mode andrey.konovalov
2021-08-12  8:56   ` Marco Elver
2021-08-11 19:23 ` [PATCH 6/8] kasan: test: clean up ksize_uaf andrey.konovalov
2021-08-12  8:56   ` Marco Elver
2021-08-11 19:30 ` [PATCH 7/8] kasan: test: avoid corrupting memory in copy_user_test andrey.konovalov
2021-08-12  8:50   ` Marco Elver
2021-08-11 19:34 ` [PATCH 8/8] kasan: test: avoid corrupting memory in kasan_rcu_uaf andrey.konovalov
2021-08-12  8:50   ` Marco Elver
2021-08-12  8:58 ` [PATCH 0/8] kasan: test: avoid crashing the kernel with HW_TAGS Marco Elver

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).