[PATCH] kasan: test: Bypass __alloc_size checks

0 views
Skip to first unread message

Kees Cook

unread,
Oct 5, 2021, 11:55:31 PMOct 5
to Andrew Morton, Kees Cook, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, kasa...@googlegroups.com, linux-...@vger.kernel.org, linux-h...@vger.kernel.org
Intentional overflows, as performed by the KASAN tests, are detected
at compile time[1] (instead of only at run-time) with the addition of
__alloc_size. Fix this by forcing the compiler into not being able to
trust the size used following the kmalloc()s.

[1] https://lore.kernel.org/lkml/20211005184717.65c6...@linux-foundation.org

Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: kasa...@googlegroups.com
Signed-off-by: Kees Cook <kees...@chromium.org>
---
lib/test_kasan.c | 10 +++++-----
lib/test_kasan_module.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 8835e0784578..0e1f8d5281b4 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -435,7 +435,7 @@ static void kmalloc_uaf_16(struct kunit *test)
static void kmalloc_oob_memset_2(struct kunit *test)
{
char *ptr;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;

ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
@@ -447,7 +447,7 @@ static void kmalloc_oob_memset_2(struct kunit *test)
static void kmalloc_oob_memset_4(struct kunit *test)
{
char *ptr;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;

ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
@@ -459,7 +459,7 @@ static void kmalloc_oob_memset_4(struct kunit *test)
static void kmalloc_oob_memset_8(struct kunit *test)
{
char *ptr;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;

ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
@@ -471,7 +471,7 @@ static void kmalloc_oob_memset_8(struct kunit *test)
static void kmalloc_oob_memset_16(struct kunit *test)
{
char *ptr;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;

ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
@@ -483,7 +483,7 @@ static void kmalloc_oob_memset_16(struct kunit *test)
static void kmalloc_oob_in_memset(struct kunit *test)
{
char *ptr;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;

ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
index 7ebf433edef3..c8cc77b1dcf3 100644
--- a/lib/test_kasan_module.c
+++ b/lib/test_kasan_module.c
@@ -19,7 +19,7 @@ static noinline void __init copy_user_test(void)
{
char *kmem;
char __user *usermem;
- size_t size = 128 - KASAN_GRANULE_SIZE;
+ volatile size_t size = 128 - KASAN_GRANULE_SIZE;
int __maybe_unused unused;

kmem = kmalloc(size, GFP_KERNEL);
--
2.30.2

Mark Rutland

unread,
Oct 6, 2021, 7:38:53 AMOct 6
to Kees Cook, Andrew Morton, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, kasa...@googlegroups.com, linux-...@vger.kernel.org, linux-h...@vger.kernel.org
Hi Kees,

On Tue, Oct 05, 2021 at 08:55:22PM -0700, Kees Cook wrote:
> Intentional overflows, as performed by the KASAN tests, are detected
> at compile time[1] (instead of only at run-time) with the addition of
> __alloc_size. Fix this by forcing the compiler into not being able to
> trust the size used following the kmalloc()s.

It might be better to use OPTIMIZER_HIDE_VAR(), since that's intended to
make the value opaque to the compiler, and volatile might not always do
that depending on how the compiler tracks the variable.

Thanks,
Mark.

Kees Cook

unread,
Oct 6, 2021, 12:33:22 PMOct 6
to Mark Rutland, Andrew Morton, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, kasa...@googlegroups.com, linux-...@vger.kernel.org, linux-h...@vger.kernel.org
On Wed, Oct 06, 2021 at 12:38:36PM +0100, Mark Rutland wrote:
> Hi Kees,
>
> On Tue, Oct 05, 2021 at 08:55:22PM -0700, Kees Cook wrote:
> > Intentional overflows, as performed by the KASAN tests, are detected
> > at compile time[1] (instead of only at run-time) with the addition of
> > __alloc_size. Fix this by forcing the compiler into not being able to
> > trust the size used following the kmalloc()s.
>
> It might be better to use OPTIMIZER_HIDE_VAR(), since that's intended to
> make the value opaque to the compiler, and volatile might not always do
> that depending on how the compiler tracks the variable.

Given both you and Jann[1] have suggested this, I'll send a v2 with that.
:) Thanks!

-Kees

[1] https://lore.kernel.org/lkml/CAG48ez19raco+s+UF8eiXqTv...@mail.gmail.com/

--
Kees Cook

Kees Cook

unread,
Oct 6, 2021, 2:15:50 PMOct 6
to Andrew Morton, Kees Cook, Andrey Ryabinin, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, kasa...@googlegroups.com, Jann Horn, Mark Rutland, linux-...@vger.kernel.org, linux-h...@vger.kernel.org
Intentional overflows, as performed by the KASAN tests, are detected
at compile time[1] (instead of only at run-time) with the addition of
__alloc_size. Fix this by forcing the compiler into not being able to
trust the size used following the kmalloc()s.

[1] https://lore.kernel.org/lkml/20211005184717.65c6...@linux-foundation.org

Cc: Andrey Ryabinin <ryabin...@gmail.com>
Cc: Alexander Potapenko <gli...@google.com>
Cc: Andrey Konovalov <andre...@gmail.com>
Cc: Dmitry Vyukov <dvy...@google.com>
Cc: kasa...@googlegroups.com
Signed-off-by: Kees Cook <kees...@chromium.org>
---
v2: use OPTIMIZER_HIDE_VAR() (jann, mark)
v1: https://lore.kernel.org/lkml/20211006035522.5...@chromium.org/
---
lib/test_kasan.c | 8 +++++++-
lib/test_kasan_module.c | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 8835e0784578..8a8a8133f4cd 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -440,6 +440,7 @@ static void kmalloc_oob_memset_2(struct kunit *test)
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

+ OPTIMIZER_HIDE_VAR(size);
KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size - 1, 0, 2));
kfree(ptr);
}
@@ -452,6 +453,7 @@ static void kmalloc_oob_memset_4(struct kunit *test)
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

+ OPTIMIZER_HIDE_VAR(size);
KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size - 3, 0, 4));
kfree(ptr);
}
@@ -464,6 +466,7 @@ static void kmalloc_oob_memset_8(struct kunit *test)
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

+ OPTIMIZER_HIDE_VAR(size);
KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size - 7, 0, 8));
kfree(ptr);
}
@@ -476,6 +479,7 @@ static void kmalloc_oob_memset_16(struct kunit *test)
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

+ OPTIMIZER_HIDE_VAR(size);
KUNIT_EXPECT_KASAN_FAIL(test, memset(ptr + size - 15, 0, 16));
kfree(ptr);
}
@@ -488,6 +492,7 @@ static void kmalloc_oob_in_memset(struct kunit *test)
ptr = kmalloc(size, GFP_KERNEL);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

+ OPTIMIZER_HIDE_VAR(size);
KUNIT_EXPECT_KASAN_FAIL(test,
memset(ptr, 0, size + KASAN_GRANULE_SIZE));
kfree(ptr);
@@ -497,7 +502,7 @@ static void kmalloc_memmove_invalid_size(struct kunit *test)
{
char *ptr;
size_t size = 64;
- volatile size_t invalid_size = -2;
+ size_t invalid_size = -2;

/*
* Hardware tag-based mode doesn't check memmove for negative size.
@@ -510,6 +515,7 @@ static void kmalloc_memmove_invalid_size(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);

memset((char *)ptr, 0, 64);
+ OPTIMIZER_HIDE_VAR(invalid_size);
KUNIT_EXPECT_KASAN_FAIL(test,
memmove((char *)ptr, (char *)ptr + 4, invalid_size));
kfree(ptr);
diff --git a/lib/test_kasan_module.c b/lib/test_kasan_module.c
index 7ebf433edef3..b112cbc835e9 100644
--- a/lib/test_kasan_module.c
+++ b/lib/test_kasan_module.c
@@ -35,6 +35,8 @@ static noinline void __init copy_user_test(void)
return;
}

+ OPTIMIZER_HIDE_VAR(size);
+
pr_info("out-of-bounds in copy_from_user()\n");
unused = copy_from_user(kmem, usermem, size + 1);

--
2.30.2

Reply all
Reply to author
Forward
0 new messages