* [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
@ 2021-05-07 2:59 ` Peter Collingbourne
0 siblings, 0 replies; 6+ messages in thread
From: Peter Collingbourne @ 2021-05-07 2:59 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko
Cc: Peter Collingbourne, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, linux-mm, stable
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.
These accesses use volatile pointers to char (char *volatile) rather
than the more conventional pointers to volatile char (volatile char *)
because we want to prevent the compiler from making inferences about
the pointer itself (i.e. its array bounds), not the data that it
refers to.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Cc: stable@vger.kernel.org
Link: https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9
---
lib/test_kasan.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index dc05cfc2d12f..cacbbbdef768 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -654,8 +654,20 @@ 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];
+ /*
+ * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
+ * from failing here and panicing the kernel, access the array via a
+ * volatile pointer, which will prevent the compiler from being able to
+ * determine the array bounds.
+ *
+ * This access uses a volatile pointer to char (char *volatile) rather
+ * than the more conventional pointer to volatile char (volatile char *)
+ * because we want to prevent the compiler from making inferences about
+ * the pointer itself (i.e. its array bounds), not the data that it
+ * refers to.
+ */
+ char *volatile array = global_array;
+ char *p = &array[ARRAY_SIZE(global_array) + 3];
/* Only generic mode instruments globals. */
KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
@@ -703,8 +715,9 @@ 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];
+ /* See comment in kasan_global_oob. */
+ 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 +728,9 @@ static void kasan_alloca_oob_left(struct kunit *test)
{
volatile int i = 10;
char alloca_array[i];
- char *p = alloca_array - 1;
+ /* See comment in kasan_global_oob. */
+ 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 +743,9 @@ static void kasan_alloca_oob_right(struct kunit *test)
{
volatile int i = 10;
char alloca_array[i];
- char *p = alloca_array + i;
+ /* See comment in kasan_global_oob. */
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
@ 2021-05-07 2:59 ` Peter Collingbourne
0 siblings, 0 replies; 6+ messages in thread
From: Peter Collingbourne @ 2021-05-07 2:59 UTC (permalink / raw)
To: Andrey Konovalov, Alexander Potapenko
Cc: Peter Collingbourne, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, linux-mm, stable
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.
These accesses use volatile pointers to char (char *volatile) rather
than the more conventional pointers to volatile char (volatile char *)
because we want to prevent the compiler from making inferences about
the pointer itself (i.e. its array bounds), not the data that it
refers to.
Signed-off-by: Peter Collingbourne <pcc@google.com>
Cc: stable@vger.kernel.org
Link: https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9
---
lib/test_kasan.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index dc05cfc2d12f..cacbbbdef768 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -654,8 +654,20 @@ 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];
+ /*
+ * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
+ * from failing here and panicing the kernel, access the array via a
+ * volatile pointer, which will prevent the compiler from being able to
+ * determine the array bounds.
+ *
+ * This access uses a volatile pointer to char (char *volatile) rather
+ * than the more conventional pointer to volatile char (volatile char *)
+ * because we want to prevent the compiler from making inferences about
+ * the pointer itself (i.e. its array bounds), not the data that it
+ * refers to.
+ */
+ char *volatile array = global_array;
+ char *p = &array[ARRAY_SIZE(global_array) + 3];
/* Only generic mode instruments globals. */
KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
@@ -703,8 +715,9 @@ 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];
+ /* See comment in kasan_global_oob. */
+ 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 +728,9 @@ static void kasan_alloca_oob_left(struct kunit *test)
{
volatile int i = 10;
char alloca_array[i];
- char *p = alloca_array - 1;
+ /* See comment in kasan_global_oob. */
+ 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 +743,9 @@ static void kasan_alloca_oob_right(struct kunit *test)
{
volatile int i = 10;
char alloca_array[i];
- char *p = alloca_array + i;
+ /* See comment in kasan_global_oob. */
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
2021-05-07 2:59 ` Peter Collingbourne
@ 2021-05-07 7:15 ` Alexander Potapenko
-1 siblings, 0 replies; 6+ messages in thread
From: Alexander Potapenko @ 2021-05-07 7:15 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Andrey Konovalov, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, Linux Memory Management List,
stable
On Fri, May 7, 2021 at 4:59 AM Peter Collingbourne <pcc@google.com> 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.
Thanks for tracking this down! These crashes have been puzzling me for a while.
> These accesses use volatile pointers to char (char *volatile) rather
> than the more conventional pointers to volatile char (volatile char *)
> because we want to prevent the compiler from making inferences about
> the pointer itself (i.e. its array bounds), not the data that it
> refers to.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
(also note you are missing the Acked-by: here that Andrey gave)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
@ 2021-05-07 7:15 ` Alexander Potapenko
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Potapenko @ 2021-05-07 7:15 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Andrey Konovalov, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, Linux Memory Management List,
stable
On Fri, May 7, 2021 at 4:59 AM Peter Collingbourne <pcc@google.com> 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.
Thanks for tracking this down! These crashes have been puzzling me for a while.
> These accesses use volatile pointers to char (char *volatile) rather
> than the more conventional pointers to volatile char (volatile char *)
> because we want to prevent the compiler from making inferences about
> the pointer itself (i.e. its array bounds), not the data that it
> refers to.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
Tested-by: Alexander Potapenko <glider@google.com>
(also note you are missing the Acked-by: here that Andrey gave)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
2021-05-07 2:59 ` Peter Collingbourne
@ 2021-05-07 13:42 ` Andrey Konovalov
-1 siblings, 0 replies; 6+ messages in thread
From: Andrey Konovalov @ 2021-05-07 13:42 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Alexander Potapenko, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, Linux Memory Management List,
stable
On Fri, May 7, 2021 at 4:59 AM Peter Collingbourne <pcc@google.com> 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.
>
> These accesses use volatile pointers to char (char *volatile) rather
> than the more conventional pointers to volatile char (volatile char *)
> because we want to prevent the compiler from making inferences about
> the pointer itself (i.e. its array bounds), not the data that it
> refers to.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Cc: stable@vger.kernel.org
> Link: https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9
> ---
> lib/test_kasan.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index dc05cfc2d12f..cacbbbdef768 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -654,8 +654,20 @@ 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];
> + /*
> + * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
> + * from failing here and panicing the kernel, access the array via a
> + * volatile pointer, which will prevent the compiler from being able to
> + * determine the array bounds.
> + *
> + * This access uses a volatile pointer to char (char *volatile) rather
> + * than the more conventional pointer to volatile char (volatile char *)
> + * because we want to prevent the compiler from making inferences about
> + * the pointer itself (i.e. its array bounds), not the data that it
> + * refers to.
> + */
> + char *volatile array = global_array;
> + char *p = &array[ARRAY_SIZE(global_array) + 3];
>
> /* Only generic mode instruments globals. */
> KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
> @@ -703,8 +715,9 @@ 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];
> + /* See comment in kasan_global_oob. */
> + 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 +728,9 @@ static void kasan_alloca_oob_left(struct kunit *test)
> {
> volatile int i = 10;
> char alloca_array[i];
> - char *p = alloca_array - 1;
> + /* See comment in kasan_global_oob. */
> + 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 +743,9 @@ static void kasan_alloca_oob_right(struct kunit *test)
> {
> volatile int i = 10;
> char alloca_array[i];
> - char *p = alloca_array + i;
> + /* See comment in kasan_global_oob. */
> + 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
>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled
@ 2021-05-07 13:42 ` Andrey Konovalov
0 siblings, 0 replies; 6+ messages in thread
From: Andrey Konovalov @ 2021-05-07 13:42 UTC (permalink / raw)
To: Peter Collingbourne
Cc: Alexander Potapenko, George Popescu, Elena Petrova,
Evgenii Stepanov, Andrew Morton, Linux Memory Management List,
stable
On Fri, May 7, 2021 at 4:59 AM Peter Collingbourne <pcc@google.com> 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.
>
> These accesses use volatile pointers to char (char *volatile) rather
> than the more conventional pointers to volatile char (volatile char *)
> because we want to prevent the compiler from making inferences about
> the pointer itself (i.e. its array bounds), not the data that it
> refers to.
>
> Signed-off-by: Peter Collingbourne <pcc@google.com>
> Cc: stable@vger.kernel.org
> Link: https://linux-review.googlesource.com/id/I90b1713fbfa1bf68ff895aef099ea77b98a7c3b9
> ---
> lib/test_kasan.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index dc05cfc2d12f..cacbbbdef768 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -654,8 +654,20 @@ 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];
> + /*
> + * Deliberate out-of-bounds access. To prevent CONFIG_UBSAN_LOCAL_BOUNDS
> + * from failing here and panicing the kernel, access the array via a
> + * volatile pointer, which will prevent the compiler from being able to
> + * determine the array bounds.
> + *
> + * This access uses a volatile pointer to char (char *volatile) rather
> + * than the more conventional pointer to volatile char (volatile char *)
> + * because we want to prevent the compiler from making inferences about
> + * the pointer itself (i.e. its array bounds), not the data that it
> + * refers to.
> + */
> + char *volatile array = global_array;
> + char *p = &array[ARRAY_SIZE(global_array) + 3];
>
> /* Only generic mode instruments globals. */
> KASAN_TEST_NEEDS_CONFIG_ON(test, CONFIG_KASAN_GENERIC);
> @@ -703,8 +715,9 @@ 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];
> + /* See comment in kasan_global_oob. */
> + 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 +728,9 @@ static void kasan_alloca_oob_left(struct kunit *test)
> {
> volatile int i = 10;
> char alloca_array[i];
> - char *p = alloca_array - 1;
> + /* See comment in kasan_global_oob. */
> + 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 +743,9 @@ static void kasan_alloca_oob_right(struct kunit *test)
> {
> volatile int i = 10;
> char alloca_array[i];
> - char *p = alloca_array + i;
> + /* See comment in kasan_global_oob. */
> + 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
>
Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-07 13:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 2:59 [PATCH v2] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled Peter Collingbourne
2021-05-07 2:59 ` Peter Collingbourne
2021-05-07 7:15 ` Alexander Potapenko
2021-05-07 7:15 ` Alexander Potapenko
2021-05-07 13:42 ` Andrey Konovalov
2021-05-07 13:42 ` Andrey Konovalov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.