Re: [PATCH] lib/string: Fix UBSAN misaligned access in sized_strscpy

[David Laight <david.laight.linux@gmail.com>]

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
> > > >
> > > >  
> > >  
> >  
>