* [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: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 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: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-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 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-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: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