kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Signed-off-by Peter Collingbourne <[email protected]>

Cc [email protected]

Link https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9

---
 lib/test_kasan.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/test_kasan.c +0 -0

index dc05cfc2d12f..2a078e8e7b8e 100644

lib/test_kasan.c +6 -5

        /* Only generic mode instruments globals. */
        KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);

@@ -703,8 +703,8 @@ static void ksize_uaf(struct kunit *test)
 static void kasan_stack_oob(struct kunit *test)
 {
        char stack_array[10];
-       volatile int i = OOB_TAG_OFF;
-       char *p = &stack_array[ARRAY_SIZE(stack_array) + i];
+       char *volatile array = stack_array;
+       char *p = &array[ARRAY_SIZE(stack_array) + OOB_TAG_OFF];
 
        KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_STACK);
 

@@ -715,7 +715,8 @@ static void kasan_alloca_oob_left(struct kunit *test)
 {
        volatile int i = 10;
        char alloca_array[i];
-       char *p = alloca_array - 1;
+       char *volatile array = alloca_array;
+       char *p = array - 1;
 
        /* Only generic mode instruments dynamic allocas. */
        KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);

@@ -728,7 +729,8 @@ static void kasan_alloca_oob_right(struct kunit *test)
 {
        volatile int i = 10;
        char alloca_array[i];
-       char *p = alloca_array + i;
+       char *volatile array = alloca_array;
+       char *p = array + i;
 
        /* Only generic mode instruments dynamic allocas. */
        KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
--

2.31.1.607.g51e8a6a459-goog

Quoted 20 lines · Peter Collingbourne

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Signed-off-by Peter Collingbourne <[email protected]>

Cc [email protected]

Link https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9

---
 lib/test_kasan.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/test_kasan.c +0 -0

index dc05cfc2d12f..2a078e8e7b8e 100644
@@ -654,8 +654,8 @@ static char global_array[10];
 

lib/test_kasan.c static char global_array[10]; · line 658

Quoted 6 lines

lib/test_kasan.c +2 -2

 static void kasan_global_oob(struct kunit *test)
 {
-       volatile int i = 3;
-       char *p = &global_array[ARRAY_SIZE(global_array) + i];
+       char *volatile array = global_array;
+       char *p = &array[ARRAY_SIZE(global_array) + 3];

Nit: in the kernel, "volatile" usually comes before the pointer type.

Quoted 22 lines · Andrey Konovalov

Quoted 20 lines · Peter Collingbourne

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Signed-off-by Peter Collingbourne <[email protected]>

Cc [email protected]

Link https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9

---
 lib/test_kasan.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/test_kasan.c +0 -0

index dc05cfc2d12f..2a078e8e7b8e 100644
@@ -654,8 +654,8 @@ static char global_array[10];
 

Quoted 8 lines

lib/test_kasan.c static char global_array[10]; · line 658

Quoted 6 lines

lib/test_kasan.c +2 -2

 static void kasan_global_oob(struct kunit *test)
 {
-       volatile int i = 3;
-       char *p = &global_array[ARRAY_SIZE(global_array) + i];
+       char *volatile array = global_array;
+       char *p = &array[ARRAY_SIZE(global_array) + 3];

Nit: in the kernel, "volatile" usually comes before the pointer type.

That would refer to a different type. "volatile char *" is a pointer to volatile char, while "char *volatile" is a volatile pointer to char. The latter is what we want here, because we want to prevent the compiler from inferring things about the pointer itself (i.e. its array bounds), not the data that it refers to.

Quoted 33 lines · Peter Collingbourne

Quoted 30 lines · Andrey Konovalov

Quoted 19 lines · Peter Collingbourne

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Signed-off-by Peter Collingbourne <[email protected]>

Cc [email protected]

Link https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9

---
 lib/test_kasan.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

lib/test_kasan.c +0 -0

index dc05cfc2d12f..2a078e8e7b8e 100644

lib/test_kasan.c static char global_array[10]; · line 658

Quoted 7 lines

lib/test_kasan.c +2 -2

@@ -654,8 +654,8 @@ static char global_array[10];
 
 static void kasan_global_oob(struct kunit *test)
 {
-       volatile int i = 3;
-       char *p = &global_array[ARRAY_SIZE(global_array) + i];
+       char *volatile array = global_array;
+       char *p = &array[ARRAY_SIZE(global_array) + 3];

Nit: in the kernel, "volatile" usually comes before the pointer type.

Quoted 5 lines

That would refer to a different type. "volatile char *" is a pointer to volatile char, while "char *volatile" is a volatile pointer to char. The latter is what we want here, because we want to prevent the compiler from inferring things about the pointer itself (i.e. its array bounds), not the data that it refers to.

I see. This is unusual. I'd say this needs to be explicitly explained in the commit message, as well as in a comment in the code.

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Quoted 5 lines · On Thu, 6 May 2021 14:20:25 -0700 Peter Collingbourne <[email protected]> wrote:

These tests deliberately access these arrays out of bounds, which will cause the dynamic local bounds checks inserted by CONFIG_UBSAN_LOCAL_BOUNDS to fail and panic the kernel. To avoid this problem, access the arrays via volatile pointers, which will prevent the compiler from being able to determine the array bounds.

Huh. Is this use of volatile the official way of suppressing the generation of the checking code or is it just something which happened to work? I'm wondering if this workaround should be formalized in some fashion (presumably a wrapper) rather than mysteriously and unexplainedly open-coding it like this.