[PATCH] string: instrument the word loads in sized_strscpy()

[PATCH] string: instrument the word loads in sized_strscpy()

[Qiliang Yuan]

sized_strscpy() copies a word at a time. The loads go through
read_word_at_a_time(), which checks the first byte of each word with
KASAN and KCSAN, or through load_unaligned_zeropad() on architectures
with CONFIG_DCACHE_WORD_ACCESS.

load_unaligned_zeropad() is inline assembly that the compiler does not
instrument, and it has no such check. Since sized_strscpy() switched to
it, generic and software tag-based KASAN no longer report strscpy()
reading from a freed or out-of-bounds source on x86_64, arm64 and the
other architectures that select CONFIG_DCACHE_WORD_ACCESS. On x86_64
the kasan_strings KUnit case, which expects a report when the first
source byte is out of bounds, fails as a result:

  KASAN and string KUnit suites      failures
  on x86_64, generic KASAN        before    after
  CONFIG_FORTIFY_SOURCE=n              1        0
  CONFIG_FORTIFY_SOURCE=y              1        0

Check the first byte with instrument_read() before each
load_unaligned_zeropad(), as read_word_at_a_time() does. Hardware
tag-based KASAN does not use this check, so it does not bring back the
tag check faults that load_unaligned_zeropad() avoids.

Fixes: d94c12bd97d5 ("string: Add load_unaligned_zeropad() code path to sized_strscpy()")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 lib/string.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/string.c b/lib/string.c
index a3778d5aab4af..91de344956fee 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -19,6 +19,7 @@
 #include <linux/bug.h>
 #include <linux/ctype.h>
 #include <linux/errno.h>
+#include <linux/instrumented.h>
 #include <linux/limits.h>
 #include <linux/linkage.h>
 #include <linux/minmax.h>
@@ -132,6 +133,12 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
 		unsigned long c, data;
 
 #ifdef CONFIG_DCACHE_WORD_ACCESS
+		/*
+		 * Unlike read_word_at_a_time(), load_unaligned_zeropad() is
+		 * not instrumented. Check the first byte, which must be
+		 * readable, as read_word_at_a_time() does.
+		 */
+		instrument_read(src + res, 1);
 		c = load_unaligned_zeropad(src+res);
 #else
 		c = read_word_at_a_time(src+res);

---
base-commit: 90201351a862182bdabfc1ad1c97b1e1b6ed5931
change-id: 20261016-bug-string-strscpy-kasan-first-byte-335585ce203d

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

Re: [PATCH] string: instrument the word loads in sized_strscpy()

[Peter Collingbourne]

On Sat, Oct 17, 2026 at 9:53 PM Qiliang Yuan <odys.yuan@gmail.com> wrote:
>
> sized_strscpy() copies a word at a time. The loads go through
> read_word_at_a_time(), which checks the first byte of each word with
> KASAN and KCSAN, or through load_unaligned_zeropad() on architectures
> with CONFIG_DCACHE_WORD_ACCESS.
>
> load_unaligned_zeropad() is inline assembly that the compiler does not
> instrument, and it has no such check. Since sized_strscpy() switched to
> it, generic and software tag-based KASAN no longer report strscpy()
> reading from a freed or out-of-bounds source on x86_64, arm64 and the
> other architectures that select CONFIG_DCACHE_WORD_ACCESS. On x86_64
> the kasan_strings KUnit case, which expects a report when the first
> source byte is out of bounds, fails as a result:
>
>   KASAN and string KUnit suites      failures
>   on x86_64, generic KASAN        before    after
>   CONFIG_FORTIFY_SOURCE=n              1        0
>   CONFIG_FORTIFY_SOURCE=y              1        0
>
> Check the first byte with instrument_read() before each
> load_unaligned_zeropad(), as read_word_at_a_time() does. Hardware
> tag-based KASAN does not use this check, so it does not bring back the
> tag check faults that load_unaligned_zeropad() avoids.
>
> Fixes: d94c12bd97d5 ("string: Add load_unaligned_zeropad() code path to sized_strscpy()")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
> ---
>  lib/string.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/lib/string.c b/lib/string.c
> index a3778d5aab4af..91de344956fee 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -19,6 +19,7 @@
>  #include <linux/bug.h>
>  #include <linux/ctype.h>
>  #include <linux/errno.h>
> +#include <linux/instrumented.h>
>  #include <linux/limits.h>
>  #include <linux/linkage.h>
>  #include <linux/minmax.h>
> @@ -132,6 +133,12 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
>                 unsigned long c, data;
>
>  #ifdef CONFIG_DCACHE_WORD_ACCESS
> +               /*
> +                * Unlike read_word_at_a_time(), load_unaligned_zeropad() is
> +                * not instrumented. Check the first byte, which must be
> +                * readable, as read_word_at_a_time() does.
> +                */
> +               instrument_read(src + res, 1);
>                 c = load_unaligned_zeropad(src+res);

It looks like at least some of the other callers of
load_unaligned_zeropad() have the same problem. Would it be better to
add the instrument_read() call to load_unaligned_zeropad()?

Peter

>  #else
>                 c = read_word_at_a_time(src+res);
>
> ---
> base-commit: 90201351a862182bdabfc1ad1c97b1e1b6ed5931
> change-id: 20261016-bug-string-strscpy-kasan-first-byte-335585ce203d
>
> Best regards,
> --
> Qiliang Yuan <odys.yuan@gmail.com>
>

Re: [PATCH] string: instrument the word loads in sized_strscpy()

[Andrew Morton]

On Sat, 17 Oct 2026 22:05:03 -0700 Peter Collingbourne <peter@pcc.me.uk> wrote:

> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -19,6 +19,7 @@
> >  #include <linux/bug.h>
> >  #include <linux/ctype.h>
> >  #include <linux/errno.h>
> > +#include <linux/instrumented.h>
> >  #include <linux/limits.h>
> >  #include <linux/linkage.h>
> >  #include <linux/minmax.h>
> > @@ -132,6 +133,12 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
> >                 unsigned long c, data;
> >
> >  #ifdef CONFIG_DCACHE_WORD_ACCESS
> > +               /*
> > +                * Unlike read_word_at_a_time(), load_unaligned_zeropad() is
> > +                * not instrumented. Check the first byte, which must be
> > +                * readable, as read_word_at_a_time() does.
> > +                */
> > +               instrument_read(src + res, 1);
> >                 c = load_unaligned_zeropad(src+res);
> 
> It looks like at least some of the other callers of
> load_unaligned_zeropad() have the same problem. Would it be better to
> add the instrument_read() call to load_unaligned_zeropad()?

Well, with this setup (Kconfig-time and runtime), does our user
actually care about performance?  Can we simply use plain old C code in
these situations and not compromise the runtime checking?

I'm not sure why we have the assembly thing, really.  If the kernel is
spending significant amounts of time doing string copies then why the
heck?