* [PATCH v2 0/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() @ 2025-03-18 21:40 Peter Collingbourne 2025-03-18 21:40 ` [PATCH v2 1/2] " Peter Collingbourne 2025-03-18 21:40 ` [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 Peter Collingbourne 0 siblings, 2 replies; 7+ messages in thread From: Peter Collingbourne @ 2025-03-18 21:40 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Kees Cook, Andy Shevchenko, Andrey Konovalov, Catalin Marinas, Mark Rutland Cc: Peter Collingbourne, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel This series fixes an issue where strscpy() would sometimes trigger a false positive KASAN report with MTE. Peter Collingbourne (1): string: Add load_unaligned_zeropad() code path to sized_strscpy() Vincenzo Frascino (1): kasan: Add strscpy() test to trigger tag fault on arm64 lib/string.c | 13 ++++++++++--- mm/kasan/kasan_test_c.c | 31 ++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 4 deletions(-) -- 2.49.0.395.g12beb8f557-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() 2025-03-18 21:40 [PATCH v2 0/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() Peter Collingbourne @ 2025-03-18 21:40 ` Peter Collingbourne 2025-03-19 3:06 ` Kees Cook 2025-03-18 21:40 ` [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 Peter Collingbourne 1 sibling, 1 reply; 7+ messages in thread From: Peter Collingbourne @ 2025-03-18 21:40 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Kees Cook, Andy Shevchenko, Andrey Konovalov, Catalin Marinas, Mark Rutland Cc: Peter Collingbourne, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, stable The call to read_word_at_a_time() in sized_strscpy() is problematic with MTE because it may trigger a tag check fault when reading across a tag granule (16 bytes) boundary. To make this code MTE compatible, let's start using load_unaligned_zeropad() on architectures where it is available (i.e. architectures that define CONFIG_DCACHE_WORD_ACCESS). Because load_unaligned_zeropad() takes care of page boundaries as well as tag granule boundaries, also disable the code preventing crossing page boundaries when using load_unaligned_zeropad(). Signed-off-by: Peter Collingbourne <pcc@google.com> Link: https://linux-review.googlesource.com/id/If4b22e43b5a4ca49726b4bf98ada827fdf755548 Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS") Cc: stable@vger.kernel.org --- v2: - new approach lib/string.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/string.c b/lib/string.c index eb4486ed40d25..b632c71df1a50 100644 --- a/lib/string.c +++ b/lib/string.c @@ -119,6 +119,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) if (count == 0 || WARN_ON_ONCE(count > INT_MAX)) return -E2BIG; +#ifndef CONFIG_DCACHE_WORD_ACCESS #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS /* * If src is unaligned, don't cross a page boundary, @@ -133,12 +134,14 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) /* If src or dest is unaligned, don't do word-at-a-time. */ if (((long) dest | (long) src) & (sizeof(long) - 1)) max = 0; +#endif #endif /* - * read_word_at_a_time() below may read uninitialized bytes after the - * trailing zero and use them in comparisons. Disable this optimization - * under KMSAN to prevent false positive reports. + * load_unaligned_zeropad() or read_word_at_a_time() below may read + * uninitialized bytes after the trailing zero and use them in + * comparisons. Disable this optimization under KMSAN to prevent + * false positive reports. */ if (IS_ENABLED(CONFIG_KMSAN)) max = 0; @@ -146,7 +149,11 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) while (max >= sizeof(unsigned long)) { unsigned long c, data; +#ifdef CONFIG_DCACHE_WORD_ACCESS + c = load_unaligned_zeropad(src+res); +#else c = read_word_at_a_time(src+res); +#endif if (has_zero(c, &data, &constants)) { data = prep_zero_mask(c, data, &constants); data = create_zero_mask(data); -- 2.49.0.395.g12beb8f557-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() 2025-03-18 21:40 ` [PATCH v2 1/2] " Peter Collingbourne @ 2025-03-19 3:06 ` Kees Cook 2025-03-19 3:35 ` Peter Collingbourne 0 siblings, 1 reply; 7+ messages in thread From: Kees Cook @ 2025-03-19 3:06 UTC (permalink / raw) To: Peter Collingbourne Cc: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Andy Shevchenko, Andrey Konovalov, Catalin Marinas, Mark Rutland, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, stable On Tue, Mar 18, 2025 at 02:40:32PM -0700, Peter Collingbourne wrote: > The call to read_word_at_a_time() in sized_strscpy() is problematic > with MTE because it may trigger a tag check fault when reading > across a tag granule (16 bytes) boundary. To make this code > MTE compatible, let's start using load_unaligned_zeropad() > on architectures where it is available (i.e. architectures that > define CONFIG_DCACHE_WORD_ACCESS). Because load_unaligned_zeropad() > takes care of page boundaries as well as tag granule boundaries, > also disable the code preventing crossing page boundaries when using > load_unaligned_zeropad(). > > Signed-off-by: Peter Collingbourne <pcc@google.com> > Link: https://linux-review.googlesource.com/id/If4b22e43b5a4ca49726b4bf98ada827fdf755548 > Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS") > Cc: stable@vger.kernel.org > --- > v2: > - new approach > > lib/string.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/lib/string.c b/lib/string.c > index eb4486ed40d25..b632c71df1a50 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -119,6 +119,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > if (count == 0 || WARN_ON_ONCE(count > INT_MAX)) > return -E2BIG; > > +#ifndef CONFIG_DCACHE_WORD_ACCESS > #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS I would prefer this were written as: #if !defined(CONFIG_DCACHE_WORD_ACCESS) && \ defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) Having 2 #ifs makes me think there is some reason for having them separable. But the logic here is for a single check. > /* > * If src is unaligned, don't cross a page boundary, > @@ -133,12 +134,14 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > /* If src or dest is unaligned, don't do word-at-a-time. */ > if (((long) dest | (long) src) & (sizeof(long) - 1)) > max = 0; > +#endif > #endif (Then no second #endif needed) > > /* > - * read_word_at_a_time() below may read uninitialized bytes after the > - * trailing zero and use them in comparisons. Disable this optimization > - * under KMSAN to prevent false positive reports. > + * load_unaligned_zeropad() or read_word_at_a_time() below may read > + * uninitialized bytes after the trailing zero and use them in > + * comparisons. Disable this optimization under KMSAN to prevent > + * false positive reports. > */ > if (IS_ENABLED(CONFIG_KMSAN)) > max = 0; > @@ -146,7 +149,11 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > while (max >= sizeof(unsigned long)) { > unsigned long c, data; > > +#ifdef CONFIG_DCACHE_WORD_ACCESS > + c = load_unaligned_zeropad(src+res); > +#else > c = read_word_at_a_time(src+res); > +#endif > if (has_zero(c, &data, &constants)) { > data = prep_zero_mask(c, data, &constants); > data = create_zero_mask(data); The rest seems good. Though I do wonder: what happens on a page boundary for read_word_at_a_time(), then? We get back zero-filled remainder? Will that hide a missing NUL terminator? As in, it's not actually there because of the end of the page/granule, but a zero was put in, so now it looks like it's been terminated and the exception got eaten? And doesn't this hide MTE faults since we can't differentiate "overran MTE tag" from "overran granule while over-reading"? -- Kees Cook ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() 2025-03-19 3:06 ` Kees Cook @ 2025-03-19 3:35 ` Peter Collingbourne 0 siblings, 0 replies; 7+ messages in thread From: Peter Collingbourne @ 2025-03-19 3:35 UTC (permalink / raw) To: Kees Cook Cc: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Andy Shevchenko, Andrey Konovalov, Catalin Marinas, Mark Rutland, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, stable On Tue, Mar 18, 2025 at 8:06 PM Kees Cook <kees@kernel.org> wrote: > > On Tue, Mar 18, 2025 at 02:40:32PM -0700, Peter Collingbourne wrote: > > The call to read_word_at_a_time() in sized_strscpy() is problematic > > with MTE because it may trigger a tag check fault when reading > > across a tag granule (16 bytes) boundary. To make this code > > MTE compatible, let's start using load_unaligned_zeropad() > > on architectures where it is available (i.e. architectures that > > define CONFIG_DCACHE_WORD_ACCESS). Because load_unaligned_zeropad() > > takes care of page boundaries as well as tag granule boundaries, > > also disable the code preventing crossing page boundaries when using > > load_unaligned_zeropad(). > > > > Signed-off-by: Peter Collingbourne <pcc@google.com> > > Link: https://linux-review.googlesource.com/id/If4b22e43b5a4ca49726b4bf98ada827fdf755548 > > Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS") > > Cc: stable@vger.kernel.org > > --- > > v2: > > - new approach > > > > lib/string.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/lib/string.c b/lib/string.c > > index eb4486ed40d25..b632c71df1a50 100644 > > --- a/lib/string.c > > +++ b/lib/string.c > > @@ -119,6 +119,7 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > > if (count == 0 || WARN_ON_ONCE(count > INT_MAX)) > > return -E2BIG; > > > > +#ifndef CONFIG_DCACHE_WORD_ACCESS > > #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS > > I would prefer this were written as: > > #if !defined(CONFIG_DCACHE_WORD_ACCESS) && \ > defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) > > Having 2 #ifs makes me think there is some reason for having them > separable. But the logic here is for a single check. There is indeed a reason for having two: there's an #else in the middle (which pertains to CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS only). > > > /* > > * If src is unaligned, don't cross a page boundary, > > @@ -133,12 +134,14 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > > /* If src or dest is unaligned, don't do word-at-a-time. */ > > if (((long) dest | (long) src) & (sizeof(long) - 1)) > > max = 0; > > +#endif > > #endif > > (Then no second #endif needed) > > > > > /* > > - * read_word_at_a_time() below may read uninitialized bytes after the > > - * trailing zero and use them in comparisons. Disable this optimization > > - * under KMSAN to prevent false positive reports. > > + * load_unaligned_zeropad() or read_word_at_a_time() below may read > > + * uninitialized bytes after the trailing zero and use them in > > + * comparisons. Disable this optimization under KMSAN to prevent > > + * false positive reports. > > */ > > if (IS_ENABLED(CONFIG_KMSAN)) > > max = 0; > > @@ -146,7 +149,11 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count) > > while (max >= sizeof(unsigned long)) { > > unsigned long c, data; > > > > +#ifdef CONFIG_DCACHE_WORD_ACCESS > > + c = load_unaligned_zeropad(src+res); > > +#else > > c = read_word_at_a_time(src+res); > > +#endif > > if (has_zero(c, &data, &constants)) { > > data = prep_zero_mask(c, data, &constants); > > data = create_zero_mask(data); > > The rest seems good. Though I do wonder: what happens on a page boundary > for read_word_at_a_time(), then? We get back zero-filled remainder? Will > that hide a missing NUL terminator? As in, it's not actually there > because of the end of the page/granule, but a zero was put in, so now > it looks like it's been terminated and the exception got eaten? And > doesn't this hide MTE faults since we can't differentiate "overran MTE > tag" from "overran granule while over-reading"? Correct. The behavior I implemented seems good enough for now IMO (and at least good enough for backports). If we did want to detect this case, we would need an alternative to "load_unaligned_zeropad" that would do something other than fill the unreadable bytes with zeros in the fault handler. For example, it could fill them with all-ones. That would prevent the loop from being terminated by the tag check fault, and we would proceed to the next granule, take another tag check fault which would recur in the fault handler and cause the bug to be detected. Peter ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 2025-03-18 21:40 [PATCH v2 0/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() Peter Collingbourne 2025-03-18 21:40 ` [PATCH v2 1/2] " Peter Collingbourne @ 2025-03-18 21:40 ` Peter Collingbourne 2025-03-20 17:25 ` Andrey Konovalov 1 sibling, 1 reply; 7+ messages in thread From: Peter Collingbourne @ 2025-03-18 21:40 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Kees Cook, Andy Shevchenko, Andrey Konovalov, Catalin Marinas, Mark Rutland Cc: Vincenzo Frascino, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, Will Deacon, Peter Collingbourne 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. 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 --- 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); kfree(ptr); /* -- 2.49.0.395.g12beb8f557-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 2025-03-18 21:40 ` [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 Peter Collingbourne @ 2025-03-20 17:25 ` Andrey Konovalov 2025-03-21 2:41 ` Peter Collingbourne 0 siblings, 1 reply; 7+ messages in thread From: Andrey Konovalov @ 2025-03-20 17:25 UTC (permalink / raw) To: Peter Collingbourne Cc: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Kees Cook, Andy Shevchenko, Catalin Marinas, Mark Rutland, Vincenzo Frascino, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, Will Deacon On Tue, Mar 18, 2025 at 10:41 PM 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. > > 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 > --- > 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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 2025-03-20 17:25 ` Andrey Konovalov @ 2025-03-21 2:41 ` Peter Collingbourne 0 siblings, 0 replies; 7+ messages in thread From: Peter Collingbourne @ 2025-03-21 2:41 UTC (permalink / raw) To: Andrey Konovalov Cc: Alexander Viro, Christian Brauner, Jan Kara, Andrew Morton, Kees Cook, Andy Shevchenko, Catalin Marinas, Mark Rutland, Vincenzo Frascino, linux-fsdevel, linux-kernel, linux-hardening, linux-arm-kernel, Will Deacon On Thu, Mar 20, 2025 at 10:25 AM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > On Tue, Mar 18, 2025 at 10:41 PM 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. > > > > 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 > > --- > > 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-21 2:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-18 21:40 [PATCH v2 0/2] string: Add load_unaligned_zeropad() code path to sized_strscpy() Peter Collingbourne 2025-03-18 21:40 ` [PATCH v2 1/2] " Peter Collingbourne 2025-03-19 3:06 ` Kees Cook 2025-03-19 3:35 ` Peter Collingbourne 2025-03-18 21:40 ` [PATCH v2 2/2] kasan: Add strscpy() test to trigger tag fault on arm64 Peter Collingbourne 2025-03-20 17:25 ` Andrey Konovalov 2025-03-21 2:41 ` Peter Collingbourne
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).