| Message ID | 20250325015617.23455-3-pcc@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | string: Add load_unaligned_zeropad() code path to sized_strscpy() | expand |
On Tue, Mar 25, 2025 at 2:56 AM Peter Collingbourne <pcc@google.com> wrote: > > From: Vincenzo Frascino <vincenzo.frascino@arm.com> > > When we invoke strscpy() with a maximum size of N bytes, it assumes > that: > - It can always read N bytes from the source. > - It always write N bytes (zero-padded) to the destination. > > On aarch64 with Memory Tagging Extension enabled if we pass an N that is > bigger then the source buffer, it triggers an MTE fault. > > Implement a KASAN KUnit test that triggers the issue with the current > implementation of read_word_at_a_time() on aarch64 with MTE enabled. I think the wording here is confusing, makes it sound like the issue is not fixed. > Cc: Will Deacon <will@kernel.org> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> > Co-developed-by: Peter Collingbourne <pcc@google.com> > Signed-off-by: Peter Collingbourne <pcc@google.com> > Link: https://linux-review.googlesource.com/id/If88e396b9e7c058c1a4b5a252274120e77b1898a > --- > v3: > - simplify test case > > v2: > - rebased > - fixed test failure > > mm/kasan/kasan_test_c.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c > index 59d673400085f..b69f66b7eda1d 100644 > --- a/mm/kasan/kasan_test_c.c > +++ b/mm/kasan/kasan_test_c.c > @@ -1570,6 +1570,7 @@ static void kasan_memcmp(struct kunit *test) > static void kasan_strings(struct kunit *test) > { > char *ptr; > + char *src; > size_t size = 24; > > /* > @@ -1581,6 +1582,18 @@ static void kasan_strings(struct kunit *test) > ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > > + src = kmalloc(KASAN_GRANULE_SIZE, GFP_KERNEL | __GFP_ZERO); > + strscpy(src, "f0cacc1a0000000", KASAN_GRANULE_SIZE); > + > + /* Please also extend the comment here to say something like: "Make sure that strscpy() does not trigger KASAN if it overreads into poisoned memory". > + * The expected size does not include the terminator '\0' > + * so it is (KASAN_GRANULE_SIZE - 2) == > + * KASAN_GRANULE_SIZE - ("initial removed character" + "\0"). > + */ > + KUNIT_EXPECT_EQ(test, KASAN_GRANULE_SIZE - 2, > + strscpy(ptr, src + 1, KASAN_GRANULE_SIZE)); > + > + kfree(src); > kfree(ptr); > > /* > -- > 2.49.0.395.g12beb8f557-goog > The code looks much better! Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>
diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c index 59d673400085f..b69f66b7eda1d 100644 --- a/mm/kasan/kasan_test_c.c +++ b/mm/kasan/kasan_test_c.c @@ -1570,6 +1570,7 @@ static void kasan_memcmp(struct kunit *test) static void kasan_strings(struct kunit *test) { char *ptr; + char *src; size_t size = 24; /* @@ -1581,6 +1582,18 @@ static void kasan_strings(struct kunit *test) ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); + src = kmalloc(KASAN_GRANULE_SIZE, GFP_KERNEL | __GFP_ZERO); + strscpy(src, "f0cacc1a0000000", KASAN_GRANULE_SIZE); + + /* + * The expected size does not include the terminator '\0' + * so it is (KASAN_GRANULE_SIZE - 2) == + * KASAN_GRANULE_SIZE - ("initial removed character" + "\0"). + */ + KUNIT_EXPECT_EQ(test, KASAN_GRANULE_SIZE - 2, + strscpy(ptr, src + 1, KASAN_GRANULE_SIZE)); + + kfree(src); kfree(ptr); /*