On Thu, Mar 20, 2025 at 10:25 AM Andrey Konovalov <andreyknvl@xxxxxxxxx> wrote: > > 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? The first 16 bytes, but yes. > 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)); Yes, something like that should work as well. Let me send a v3. Peter > 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 > >
