On Tue, Mar 18, 2025 at 10:41 PM Peter Collingbourne <pcc@xxxxxxxxxx> wrote: > > From: Vincenzo Frascino <vincenzo.frascino@xxxxxxx> > > 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. > > Cc: Will Deacon <will@xxxxxxxxxx> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx> > Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx> > Co-developed-by: Peter Collingbourne <pcc@xxxxxxxxxx> > Signed-off-by: Peter Collingbourne <pcc@xxxxxxxxxx> > Link: https://linux-review.googlesource.com/id/If88e396b9e7c058c1a4b5a252274120e77b1898a > --- > v2: > - rebased > - fixed test failure > > mm/kasan/kasan_test_c.c | 31 ++++++++++++++++++++++++++++++- > 1 file changed, 30 insertions(+), 1 deletion(-) > > diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c > index 59d673400085f..c4bb3ee497b54 100644 > --- a/mm/kasan/kasan_test_c.c > +++ b/mm/kasan/kasan_test_c.c > @@ -1570,7 +1570,9 @@ static void kasan_memcmp(struct kunit *test) > static void kasan_strings(struct kunit *test) > { > char *ptr; > - size_t size = 24; > + char *src, *src2; > + u8 tag; > + size_t size = 2 * KASAN_GRANULE_SIZE; > > /* > * str* functions are not instrumented with CONFIG_AMD_MEM_ENCRYPT. > @@ -1581,6 +1583,33 @@ static void kasan_strings(struct kunit *test) > ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr); > > + src = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > + strscpy(src, "f0cacc1a00000000f0cacc1a00000000", size); > + > + tag = get_tag(src); > + > + src2 = src + KASAN_GRANULE_SIZE; > + > + /* > + * Shorten string and poison the granule after it so that the unaligned > + * read in strscpy() triggers a tag mismatch. > + */ > + src[KASAN_GRANULE_SIZE - 1] = '\0'; > + kasan_poison(src2, KASAN_GRANULE_SIZE, tag + 1, false); > + > + /* > + * 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, size)); > + > + /* Undo operations above. */ > + src[KASAN_GRANULE_SIZE - 1] = '0'; > + kasan_poison(src2, KASAN_GRANULE_SIZE, tag, false); > + > + kfree(src); I have trouble understanding what this code is doing... So the goal is to call strcpy with such an address, that the first 8 bytes (partially) cover 2 granules, one accessible and the other is not? If so, can we not do something like: src = kmalloc(KASAN_GRANULE_SIZE, GFP_KERNEL | __GFP_ZERO); strscpy(src, "aabbcceeddeeffg\0", size); strscpy(ptr, src + KASAN_GRANULE_SIZE - 2, sizeof(unsigned long)); Otherwise, this code needs more explanatory comments and it's probably better to move it out to a helper function. > kfree(ptr); > > /* > -- > 2.49.0.395.g12beb8f557-goog >
