public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
@ 2026-02-24 17:04 Fuad Tabba
  2026-02-24 17:21 ` Andy Shevchenko
  2026-02-24 21:07 ` Kees Cook
  0 siblings, 2 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-02-24 17:04 UTC (permalink / raw)
  To: Kees Cook, Andy Shevchenko, Andrew Morton
  Cc: linux-hardening, linux-kernel, will, tabba

sized_strscpy() performs word-at-a-time writes to the destination
buffer. If the destination buffer is not aligned to unsigned long,
direct assignment causes UBSAN misaligned-access errors.

Use put_unaligned() to safely write the words to the destination.

Fixes: 30035e45753b7 ("string: provide strscpy()")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 lib/string.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index b632c71df1a5..a1697bf72078 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -157,16 +157,16 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
 		if (has_zero(c, &data, &constants)) {
 			data = prep_zero_mask(c, data, &constants);
 			data = create_zero_mask(data);
-			*(unsigned long *)(dest+res) = c & zero_bytemask(data);
+			put_unaligned(c & zero_bytemask(data), (unsigned long *)(dest+res));
 			return res + find_zero(data);
 		}
 		count -= sizeof(unsigned long);
 		if (unlikely(!count)) {
 			c &= ALLBUTLAST_BYTE_MASK;
-			*(unsigned long *)(dest+res) = c;
+			put_unaligned(c, (unsigned long *)(dest+res));
 			return -E2BIG;
 		}
-		*(unsigned long *)(dest+res) = c;
+		put_unaligned(c, (unsigned long *)(dest+res));
 		res += sizeof(unsigned long);
 		max -= sizeof(unsigned long);
 	}
-- 
2.53.0.371.g1d285c8824-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 17:04 [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy Fuad Tabba
@ 2026-02-24 17:21 ` Andy Shevchenko
  2026-02-24 17:54   ` Fuad Tabba
  2026-02-24 21:07 ` Kees Cook
  1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2026-02-24 17:21 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Kees Cook, Andy Shevchenko, Andrew Morton, linux-hardening,
	linux-kernel, will

On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> sized_strscpy() performs word-at-a-time writes to the destination
> buffer. If the destination buffer is not aligned to unsigned long,
> direct assignment causes UBSAN misaligned-access errors.
> 
> Use put_unaligned() to safely write the words to the destination.

Have you measured the performance impact?
Have you read the comment near to

        if (IS_ENABLED(CONFIG_KMSAN))

?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 17:21 ` Andy Shevchenko
@ 2026-02-24 17:54   ` Fuad Tabba
  2026-02-24 23:06     ` David Laight
  0 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-02-24 17:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Kees Cook, Andy Shevchenko, Andrew Morton, linux-hardening,
	linux-kernel, will

Hi Andy,

On Tue, 24 Feb 2026 at 17:21, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> > sized_strscpy() performs word-at-a-time writes to the destination
> > buffer. If the destination buffer is not aligned to unsigned long,
> > direct assignment causes UBSAN misaligned-access errors.
> >
> > Use put_unaligned() to safely write the words to the destination.
>
> Have you measured the performance impact?

Not directly. I verified the disassembly for both x86_64 and aarch64.
On x86_64, both the raw pointer cast and put_unaligned() compile down
to mov %rdi,(%rsi). On aarch64, both compile to str x0, [x1].

> Have you read the comment near to
>
>         if (IS_ENABLED(CONFIG_KMSAN))

Not until now to be honest. However, are you asking whether
put_unaligned() breaks KMSAN? I don't think it does, max is set to 0
when KMSAN is enabled, this entire while loop is bypassed.

Thanks,
/fuad

> ?
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 17:04 [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy Fuad Tabba
  2026-02-24 17:21 ` Andy Shevchenko
@ 2026-02-24 21:07 ` Kees Cook
  2026-02-25  8:08   ` Fuad Tabba
  1 sibling, 1 reply; 11+ messages in thread
From: Kees Cook @ 2026-02-24 21:07 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Andy Shevchenko, Andrew Morton, linux-hardening, linux-kernel,
	will

On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> sized_strscpy() performs word-at-a-time writes to the destination
> buffer. If the destination buffer is not aligned to unsigned long,
> direct assignment causes UBSAN misaligned-access errors.

Is this via CONFIG_UBSAN_ALIGNMENT=y ? Note this in the Kconfig:

          Enabling this option on architectures that support unaligned
          accesses may produce a lot of false positives.

which architecture are you checking this on?

> Use put_unaligned() to safely write the words to the destination.

Also, I thought the word-at-a-time work in sized_strscpy() was
specifically to take advantage of aligned word writes? This doesn't seem
like the right solution, and I think we're already disabling the
unaligned access by using "max=0" in the earlier checks.

I think the bug may be that you got CONFIG_UBSAN_ALIGNMENT enabled for
an arch that doesn't suffer from unaligned access problems. :) We should
fix the Kconfig!

-Kees

> 
> Fixes: 30035e45753b7 ("string: provide strscpy()")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  lib/string.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/string.c b/lib/string.c
> index b632c71df1a5..a1697bf72078 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -157,16 +157,16 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
>  		if (has_zero(c, &data, &constants)) {
>  			data = prep_zero_mask(c, data, &constants);
>  			data = create_zero_mask(data);
> -			*(unsigned long *)(dest+res) = c & zero_bytemask(data);
> +			put_unaligned(c & zero_bytemask(data), (unsigned long *)(dest+res));
>  			return res + find_zero(data);
>  		}
>  		count -= sizeof(unsigned long);
>  		if (unlikely(!count)) {
>  			c &= ALLBUTLAST_BYTE_MASK;
> -			*(unsigned long *)(dest+res) = c;
> +			put_unaligned(c, (unsigned long *)(dest+res));
>  			return -E2BIG;
>  		}
> -		*(unsigned long *)(dest+res) = c;
> +		put_unaligned(c, (unsigned long *)(dest+res));
>  		res += sizeof(unsigned long);
>  		max -= sizeof(unsigned long);
>  	}
> -- 
> 2.53.0.371.g1d285c8824-goog
> 
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 17:54   ` Fuad Tabba
@ 2026-02-24 23:06     ` David Laight
  2026-02-25  8:33       ` Fuad Tabba
  0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2026-02-24 23:06 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Andy Shevchenko, Kees Cook, Andy Shevchenko, Andrew Morton,
	linux-hardening, linux-kernel, will

On Tue, 24 Feb 2026 17:54:07 +0000
Fuad Tabba <tabba@google.com> wrote:

> Hi Andy,
> 
> On Tue, 24 Feb 2026 at 17:21, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:  
> > > sized_strscpy() performs word-at-a-time writes to the destination
> > > buffer. If the destination buffer is not aligned to unsigned long,
> > > direct assignment causes UBSAN misaligned-access errors.
> > >
> > > Use put_unaligned() to safely write the words to the destination.  
> >
> > Have you measured the performance impact?  
> 
> Not directly. I verified the disassembly for both x86_64 and aarch64.
> On x86_64, both the raw pointer cast and put_unaligned() compile down
> to mov %rdi,(%rsi). On aarch64, both compile to str x0, [x1].

What happens on cpu that trap misaligned accesses (eg sparc64)?
put_unaligned() exists because it can be horrid.

	David

> 
> > Have you read the comment near to
> >
> >         if (IS_ENABLED(CONFIG_KMSAN))  
> 
> Not until now to be honest. However, are you asking whether
> put_unaligned() breaks KMSAN? I don't think it does, max is set to 0
> when KMSAN is enabled, this entire while loop is bypassed.
> 
> Thanks,
> /fuad
> 
> > ?
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >  
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 21:07 ` Kees Cook
@ 2026-02-25  8:08   ` Fuad Tabba
  2026-02-25  9:32     ` Andy Shevchenko
  2026-02-25 14:40     ` David Laight
  0 siblings, 2 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-02-25  8:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andy Shevchenko, Andrew Morton, linux-hardening, linux-kernel,
	will

Hi Kees,

On Tue, 24 Feb 2026 at 21:07, Kees Cook <kees@kernel.org> wrote:
>
> On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> > sized_strscpy() performs word-at-a-time writes to the destination
> > buffer. If the destination buffer is not aligned to unsigned long,
> > direct assignment causes UBSAN misaligned-access errors.
>
> Is this via CONFIG_UBSAN_ALIGNMENT=y ? Note this in the Kconfig:
>
>           Enabling this option on architectures that support unaligned
>           accesses may produce a lot of false positives.
>
> which architecture are you checking this on?

This is with CONFIG_UBSAN_ALIGNMENT=y on arm64. Although the
architecture supports unaligned accesses, I was running the UBSAN
checks (including the alignment ones) the other day while debugging an
unrelated issue. That said, the alignment checks ensure C standard
compliance and prevent the compiler from optimizing unaligned UB casts
into alignment-strict instructions (like ldp/stp or vector
instructions on arm64, which cause hardware faults).

> > Use put_unaligned() to safely write the words to the destination.
>
> Also, I thought the word-at-a-time work in sized_strscpy() was
> specifically to take advantage of aligned word writes? This doesn't seem
> like the right solution, and I think we're already disabling the
> unaligned access by using "max=0" in the earlier checks.

The max=0 check is heavily guarded. Both x86 and arm64 select
CONFIG_DCACHE_WORD_ACCESS, bypassing it:

#ifndef CONFIG_DCACHE_WORD_ACCESS
// ... alignment checks that set max = 0 ...
#endif

I also noticed that the read path already expects and handles
unaligned addresses. If you look at load_unaligned_zeropad() (called
above the write), it explicitly loads an unaligned word and handles
potential page-crossing faults. The write path lacked the equivalent
put_unaligned() wrapper, leaving it exposed to UB.

I checked the disassembly on both x86 and aarch64: put_unaligned()
(via __builtin_memcpy) compiles to the same instructions (mov and
str), preserving the optimization while making the code UBSAN-clean.

> I think the bug may be that you got CONFIG_UBSAN_ALIGNMENT enabled for
> an arch that doesn't suffer from unaligned access problems. :) We should
> fix the Kconfig!

Does that reasoning make sense for keeping the fix here rather than in
the Kconfig?

Cheers,
/fuad

> -Kees
>
> >
> > Fixes: 30035e45753b7 ("string: provide strscpy()")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  lib/string.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/string.c b/lib/string.c
> > index b632c71df1a5..a1697bf72078 100644
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -157,16 +157,16 @@ ssize_t sized_strscpy(char *dest, const char *src, size_t count)
> >               if (has_zero(c, &data, &constants)) {
> >                       data = prep_zero_mask(c, data, &constants);
> >                       data = create_zero_mask(data);
> > -                     *(unsigned long *)(dest+res) = c & zero_bytemask(data);
> > +                     put_unaligned(c & zero_bytemask(data), (unsigned long *)(dest+res));
> >                       return res + find_zero(data);
> >               }
> >               count -= sizeof(unsigned long);
> >               if (unlikely(!count)) {
> >                       c &= ALLBUTLAST_BYTE_MASK;
> > -                     *(unsigned long *)(dest+res) = c;
> > +                     put_unaligned(c, (unsigned long *)(dest+res));
> >                       return -E2BIG;
> >               }
> > -             *(unsigned long *)(dest+res) = c;
> > +             put_unaligned(c, (unsigned long *)(dest+res));
> >               res += sizeof(unsigned long);
> >               max -= sizeof(unsigned long);
> >       }
> > --
> > 2.53.0.371.g1d285c8824-goog
> >
> >
>
> --
> Kees Cook

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-24 23:06     ` David Laight
@ 2026-02-25  8:33       ` Fuad Tabba
  2026-02-25 10:11         ` David Laight
  0 siblings, 1 reply; 11+ messages in thread
From: Fuad Tabba @ 2026-02-25  8:33 UTC (permalink / raw)
  To: David Laight
  Cc: Andy Shevchenko, Kees Cook, Andy Shevchenko, Andrew Morton,
	linux-hardening, linux-kernel, will

Hi David,

On Tue, 24 Feb 2026 at 23:06, David Laight <david.laight.linux@gmail.com> wrote:
>
> On Tue, 24 Feb 2026 17:54:07 +0000
> Fuad Tabba <tabba@google.com> wrote:
>
> > Hi Andy,
> >
> > On Tue, 24 Feb 2026 at 17:21, Andy Shevchenko
> > <andriy.shevchenko@intel.com> wrote:
> > >
> > > On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> > > > sized_strscpy() performs word-at-a-time writes to the destination
> > > > buffer. If the destination buffer is not aligned to unsigned long,
> > > > direct assignment causes UBSAN misaligned-access errors.
> > > >
> > > > Use put_unaligned() to safely write the words to the destination.
> > >
> > > Have you measured the performance impact?
> >
> > Not directly. I verified the disassembly for both x86_64 and aarch64.
> > On x86_64, both the raw pointer cast and put_unaligned() compile down
> > to mov %rdi,(%rsi). On aarch64, both compile to str x0, [x1].
>
> What happens on cpu that trap misaligned accesses (eg sparc64)?
> put_unaligned() exists because it can be horrid.

To be honest, I hadn't considered this until now. But looking at it, I
believe that the existing guards in sized_strscpy() already protect
architectures like sparc from the unaligned paths you are concerned
about. Looking at the code and configs, these do not select
CONFIG_DCACHE_WORD_ACCESS nor CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
Because of this, they fall into the #else block early in
sized_strscpy():

#ifndef CONFIG_DCACHE_WORD_ACCESS
#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
        ...
#else
        /* 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

If either dest or src is unaligned, max is set to 0. This bypasses the
loop with put_unaligned(). I checked by compiling this for sparc: if
aligned, the compiler sees that, and optimizes it into an 8-byte store
(stx %i0, [%i1]), identical to the raw pointer cast.

So this patch shouldn't introduce memcpy fallback penalties on sparc,
but it still fixes the UB on architectures like x86 and arm64.

Cheers,
/fuad

>         David
>
> >
> > > Have you read the comment near to
> > >
> > >         if (IS_ENABLED(CONFIG_KMSAN))
> >
> > Not until now to be honest. However, are you asking whether
> > put_unaligned() breaks KMSAN? I don't think it does, max is set to 0
> > when KMSAN is enabled, this entire while loop is bypassed.
> >
> > Thanks,
> > /fuad
> >
> > > ?
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> > >
> > >
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-25  8:08   ` Fuad Tabba
@ 2026-02-25  9:32     ` Andy Shevchenko
  2026-02-25 14:40     ` David Laight
  1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-02-25  9:32 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Kees Cook, Andy Shevchenko, Andrew Morton, linux-hardening,
	linux-kernel, will

On Wed, Feb 25, 2026 at 08:08:34AM +0000, Fuad Tabba wrote:
> On Tue, 24 Feb 2026 at 21:07, Kees Cook <kees@kernel.org> wrote:
> > On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:
> > > sized_strscpy() performs word-at-a-time writes to the destination
> > > buffer. If the destination buffer is not aligned to unsigned long,
> > > direct assignment causes UBSAN misaligned-access errors.
> >
> > Is this via CONFIG_UBSAN_ALIGNMENT=y ? Note this in the Kconfig:
> >
> >           Enabling this option on architectures that support unaligned
> >           accesses may produce a lot of false positives.
> >
> > which architecture are you checking this on?
> 
> This is with CONFIG_UBSAN_ALIGNMENT=y on arm64. Although the
> architecture supports unaligned accesses, I was running the UBSAN
> checks (including the alignment ones) the other day while debugging an
> unrelated issue. That said, the alignment checks ensure C standard
> compliance and prevent the compiler from optimizing unaligned UB casts
> into alignment-strict instructions (like ldp/stp or vector
> instructions on arm64, which cause hardware faults).
> 
> > > Use put_unaligned() to safely write the words to the destination.
> >
> > Also, I thought the word-at-a-time work in sized_strscpy() was
> > specifically to take advantage of aligned word writes? This doesn't seem
> > like the right solution, and I think we're already disabling the
> > unaligned access by using "max=0" in the earlier checks.
> 
> The max=0 check is heavily guarded. Both x86 and arm64 select
> CONFIG_DCACHE_WORD_ACCESS, bypassing it:
> 
> #ifndef CONFIG_DCACHE_WORD_ACCESS
> // ... alignment checks that set max = 0 ...
> #endif
> 
> I also noticed that the read path already expects and handles
> unaligned addresses. If you look at load_unaligned_zeropad() (called
> above the write), it explicitly loads an unaligned word and handles
> potential page-crossing faults. The write path lacked the equivalent
> put_unaligned() wrapper, leaving it exposed to UB.

Probably it needs to be reworked differently to provide write_at_a_time()
helper?

> I checked the disassembly on both x86 and aarch64: put_unaligned()
> (via __builtin_memcpy) compiles to the same instructions (mov and
> str), preserving the optimization while making the code UBSAN-clean.

You need to check this on _all_ supported architectures with all possible
related configuration option combinations.

> > I think the bug may be that you got CONFIG_UBSAN_ALIGNMENT enabled for
> > an arch that doesn't suffer from unaligned access problems. :) We should
> > fix the Kconfig!
> 
> Does that reasoning make sense for keeping the fix here rather than in
> the Kconfig?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-25  8:33       ` Fuad Tabba
@ 2026-02-25 10:11         ` David Laight
  2026-02-25 11:09           ` Fuad Tabba
  0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2026-02-25 10:11 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Andy Shevchenko, Kees Cook, Andy Shevchenko, Andrew Morton,
	linux-hardening, linux-kernel, will

On Wed, 25 Feb 2026 08:33:01 +0000
Fuad Tabba <tabba@google.com> wrote:

> Hi David,
> 
> On Tue, 24 Feb 2026 at 23:06, David Laight <david.laight.linux@gmail.com> wrote:
> >
> > On Tue, 24 Feb 2026 17:54:07 +0000
> > Fuad Tabba <tabba@google.com> wrote:
> >  
> > > Hi Andy,
> > >
> > > On Tue, 24 Feb 2026 at 17:21, Andy Shevchenko
> > > <andriy.shevchenko@intel.com> wrote:  
> > > >
> > > > On Tue, Feb 24, 2026 at 05:04:27PM +0000, Fuad Tabba wrote:  
> > > > > sized_strscpy() performs word-at-a-time writes to the destination
> > > > > buffer. If the destination buffer is not aligned to unsigned long,
> > > > > direct assignment causes UBSAN misaligned-access errors.
> > > > >
> > > > > Use put_unaligned() to safely write the words to the destination.  
> > > >
> > > > Have you measured the performance impact?  
> > >
> > > Not directly. I verified the disassembly for both x86_64 and aarch64.
> > > On x86_64, both the raw pointer cast and put_unaligned() compile down
> > > to mov %rdi,(%rsi). On aarch64, both compile to str x0, [x1].  
> >
> > What happens on cpu that trap misaligned accesses (eg sparc64)?
> > put_unaligned() exists because it can be horrid.  
> 
> To be honest, I hadn't considered this until now. But looking at it, I
> believe that the existing guards in sized_strscpy() already protect
> architectures like sparc from the unaligned paths you are concerned
> about. Looking at the code and configs, these do not select
> CONFIG_DCACHE_WORD_ACCESS nor CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS.
> Because of this, they fall into the #else block early in
> sized_strscpy():
> 
> #ifndef CONFIG_DCACHE_WORD_ACCESS
> #ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>         ...
> #else
>         /* 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
> 
> If either dest or src is unaligned, max is set to 0. This bypasses the
> loop with put_unaligned(). I checked by compiling this for sparc: if
> aligned, the compiler sees that, and optimizes it into an 8-byte store
> (stx %i0, [%i1]), identical to the raw pointer cast.

That very much depends on the exactly how get/put_unaligned are implemented
(and the behaviour of the compiler).
ISTR something about not using 'casts to packed types' for the them,
which might cause the compiler to generate other code.
(Brain can't quite remember...)

	David

> 
> So this patch shouldn't introduce memcpy fallback penalties on sparc,
> but it still fixes the UB on architectures like x86 and arm64.
> 
> Cheers,
> /fuad
> 
> >         David
> >  
> > >  
> > > > Have you read the comment near to
> > > >
> > > >         if (IS_ENABLED(CONFIG_KMSAN))  
> > >
> > > Not until now to be honest. However, are you asking whether
> > > put_unaligned() breaks KMSAN? I don't think it does, max is set to 0
> > > when KMSAN is enabled, this entire while loop is bypassed.
> > >
> > > Thanks,
> > > /fuad
> > >  
> > > > ?
> > > >
> > > > --
> > > > With Best Regards,
> > > > Andy Shevchenko
> > > >
> > > >  
> > >  
> >  
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-25 10:11         ` David Laight
@ 2026-02-25 11:09           ` Fuad Tabba
  0 siblings, 0 replies; 11+ messages in thread
From: Fuad Tabba @ 2026-02-25 11:09 UTC (permalink / raw)
  To: David Laight, Andy Shevchenko
  Cc: Kees Cook, Andy Shevchenko, Andrew Morton, linux-hardening,
	linux-kernel, will

Hi,

[Andy]
> You need to check this on _all_ supported architectures with all possible
> related configuration option combinations.

[David]
> That very much depends on the exactly how get/put_unaligned are implemented
> (and the behaviour of the compiler).
> ISTR something about not using 'casts to packed types' for the them,
> which might cause the compiler to generate other code.
> (Brain can't quite remember...)

Fair points all around. To be honest, proving the behavior across all
supported architecture/config combinations is more than I can chew on
right now.

I initially stumbled across this UBSAN splat on arm64 while debugging
an unrelated issue, and I thought a targeted put_unaligned() swap
would be a straightforward fix. Given the complexity of the
architectural and compiler quirks you've raised, I agree this needs a
much deeper investigation, or potentially a new write_word_at_a_time()
abstraction, as Andy suggested.

I'll drop this patch for the time being. If I have the bandwidth in
the future, and if this splat starts causing real problems, I might
give it another go.

Thanks for the thorough review and insights! At least I learned a bit from this.

Cheers,
/fuad

>         David
>
> >
> > So this patch shouldn't introduce memcpy fallback penalties on sparc,
> > but it still fixes the UB on architectures like x86 and arm64.
> >
> > Cheers,
> > /fuad
> >
> > >         David
> > >
> > > >
> > > > > Have you read the comment near to
> > > > >
> > > > >         if (IS_ENABLED(CONFIG_KMSAN))
> > > >
> > > > Not until now to be honest. However, are you asking whether
> > > > put_unaligned() breaks KMSAN? I don't think it does, max is set to 0
> > > > when KMSAN is enabled, this entire while loop is bypassed.
> > > >
> > > > Thanks,
> > > > /fuad
> > > >
> > > > > ?
> > > > >
> > > > > --
> > > > > With Best Regards,
> > > > > Andy Shevchenko
> > > > >
> > > > >
> > > >
> > >
> >
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy
  2026-02-25  8:08   ` Fuad Tabba
  2026-02-25  9:32     ` Andy Shevchenko
@ 2026-02-25 14:40     ` David Laight
  1 sibling, 0 replies; 11+ messages in thread
From: David Laight @ 2026-02-25 14:40 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: Kees Cook, Andy Shevchenko, Andrew Morton, linux-hardening,
	linux-kernel, will

On Wed, 25 Feb 2026 08:08:34 +0000
Fuad Tabba <tabba@google.com> wrote:

...
> I also noticed that the read path already expects and handles
> unaligned addresses. If you look at load_unaligned_zeropad() (called
> above the write), it explicitly loads an unaligned word and handles
> potential page-crossing faults. The write path lacked the equivalent
> put_unaligned() wrapper, leaving it exposed to UB.

Not really, the read side is doing reads that might go past the '\0'
that terminates the string.
If they are misaligned they can fault even though the string is
correctly terminated.
OTOH the write side must not write beyond the terminating '\0'
so the memory must always be there.

I didn't look at exactly how the 'word at a time' version terminates.
For strlen() doing it at all is pretty marginal for 32bit.
For strcpy() it may depend on whether the byte writes get merged
it the cpu's store buffer.
On 64bit you have to try quite hard to stop the compiler making
'a right pigs breakfast' of generating the constants; it is pretty
bad on x64-64, mips64 is a lot worse.
On LE with fast 'bit scan' you can quickly determine the number of
partial bytes - so they can be written without re-reading from
memory.

The actual problem with that version of strlen() is that it is
only faster for strings above (about and IIRC) 64 bytes long.
So in the kernel it is pretty much a complete waste of time.
The same is probably true for strscpy().

My guess is that the fastest code uses the 'unrolled once' loop:
	do {
		if ((dst[len] = src[len]) == 0)
			break;
		if ((dst[len + 1] = src[len + 1]) == 0) {
			len++;
			break;
		}
	} while ((len += 2) < lim);
(Provided it gets compiled reasonably).
Without the writes that was pretty much the best strlen() on the few cpu
I tried it on.

	David

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-02-25 14:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 17:04 [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy Fuad Tabba
2026-02-24 17:21 ` Andy Shevchenko
2026-02-24 17:54   ` Fuad Tabba
2026-02-24 23:06     ` David Laight
2026-02-25  8:33       ` Fuad Tabba
2026-02-25 10:11         ` David Laight
2026-02-25 11:09           ` Fuad Tabba
2026-02-24 21:07 ` Kees Cook
2026-02-25  8:08   ` Fuad Tabba
2026-02-25  9:32     ` Andy Shevchenko
2026-02-25 14:40     ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox