riscv: lib: Fix address overflow due to large count values in strnlen ZBB path

[PATCH] riscv: lib: Fix address overflow due to large count values in strnlen ZBB path

Posted by gao.rui@zte.com.cn 1 month, 1 week ago

Hi all,

This patch fixes an issue in the RISC-V ZBB optimized strnlen implementation.

Problem description: When using the ZBB optimized strnlen implementation, passing very large count values (such as SIZE_MAX) can cause address overflow and return incorrect results.

This issue was observed in device-mapper tests:

sh-5.2# dmsetup create testname9 --table "0 8 zero"
sh-5.2# cat /sys/block/dm-*/dm/name
testname
sh-5.2# dmsetup remove testname9

The overflow in strnlen caused failures in string handling during device-mapper operations.

Patch summary:
- Explicitly introduce the strnlen_generic label.
- Simplify the generic implementation loop.
- Add fallback logic in strnlen_zbb to redirect to the generic path when a0 + a1 overflows.
This ensures correct behavior for large count values and SIZE_MAX cases.

Fixes: 5ba15d419fab ("riscv: lib: add strnlen() implementation")
Signed-off-by: Gao Rui <gao.rui@zte.com.cn>
---
 arch/riscv/lib/strnlen.S | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/lib/strnlen.S b/arch/riscv/lib/strnlen.S
index a8911605c248..04016e51e8b5 100644
--- a/arch/riscv/lib/strnlen.S
+++ b/arch/riscv/lib/strnlen.S
@@ -17,6 +17,7 @@ SYM_FUNC_START(strnlen)
 	__ALTERNATIVE_CFG("nop", "j strnlen_zbb", 0, RISCV_ISA_EXT_ZBB,
 		IS_ENABLED(CONFIG_RISCV_ISA_ZBB) && IS_ENABLED(CONFIG_TOOLCHAIN_HAS_ZBB))

+strnlen_generic:

 	/*
 	 * Returns
@@ -27,15 +28,17 @@ SYM_FUNC_START(strnlen)
 	 *   a1 - Max length of string
 	 *
 	 * Clobbers
-	 *   t0, t1, t2
+	 *   t0, t1
 	 */
-	addi	t1, a0, -1
-	add	t2, a0, a1
+	mv	t1, a0
+
 1:
-	addi	t1, t1, 1
-	beq	t1, t2, 2f
+	beqz	a1, 2f
+	addi	a1, a1, -1
 	lbu	t0, 0(t1)
-	bnez	t0, 1b
+	beqz	t0, 2f
+	addi	t1, t1, 1
+	j	1b
 2:
 	sub	a0, t1, a0
 	ret
@@ -73,6 +76,13 @@ strnlen_zbb:
 	/* If maxlen is 0, return 0. */
 	beqz	a1, 3f

+	/*
+	 * Fallback to generic implementation when count is large enough to
+	 * cause address overflow in the ZBB optimized path
+	 */
+	add	t4, a0, a1
+	bltu	t4, a0, strnlen_generic		/* a0 + a1 overflow */
+
 	/* Number of irrelevant bytes in the first word. */
 	andi	t2, a0, SZREG-1

-- 
2.27.0

Re: [PATCH] riscv: lib: Fix address overflow due to large count values in strnlen ZBB path

Posted by Aurelien Jarno 1 month, 1 week ago

Hi,

On 2026-08-19 16:18, gao.rui@zte.com.cn wrote:
> Hi all,
> 
> This patch fixes an issue in the RISC-V ZBB optimized strnlen implementation.
> 
> Problem description: When using the ZBB optimized strnlen implementation, passing very large count values (such as SIZE_MAX) can cause address overflow and return incorrect results.
> 
> This issue was observed in device-mapper tests:
> 
> sh-5.2# dmsetup create testname9 --table "0 8 zero"
> sh-5.2# cat /sys/block/dm-*/dm/name
> testname
> sh-5.2# dmsetup remove testname9
> 
> The overflow in strnlen caused failures in string handling during device-mapper operations.
> 
> Patch summary:
> - Explicitly introduce the strnlen_generic label.
> - Simplify the generic implementation loop.

This part should be in a separate patch, separated from the bug fix, and 
if possible with some benchmark.

> - Add fallback logic in strnlen_zbb to redirect to the generic path when a0 + a1 overflows.
> This ensures correct behavior for large count values and SIZE_MAX cases.

Is there a way to instead to fix the strnlen_zbb to avoid the fallback?

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

Re: [PATCH] riscv: lib: Fix address overflow due to large count values in strnlen ZBB path

Posted by gao.rui@zte.com.cn 1 month ago

Hi Aurelien,

>On 2026-08-19 16:18, gao.rui@zte.com.cn wrote:
>> Hi all,
>> 
>> This patch fixes an issue in the RISC-V ZBB optimized strnlen implementation.
>> 
>> Problem description: When using the ZBB optimized strnlen implementation, passing very large count values (such as SIZE_MAX) can cause address overflow and return incorrect results.
>> 
>> This issue was observed in device-mapper tests:
>> 
>> sh-5.2# dmsetup create testname9 --table "0 8 zero"
>> sh-5.2# cat /sys/block/dm-*/dm/name
>> testname
>> sh-5.2# dmsetup remove testname9
>> 
>> The overflow in strnlen caused failures in string handling during device-mapper operations.
>> 
>> Patch summary:
>> - Explicitly introduce the strnlen_generic label.
>> - Simplify the generic implementation loop.
>
>This part should be in a separate patch, separated from the bug fix, and 
>if possible with some benchmark.

I agree, I will split the generic implementation changes into a separate patch and provide benchmark results 
to show the performance impact.

>> - Add fallback logic in strnlen_zbb to redirect to the generic path when a0 + a1 overflows.
>> This ensures correct behavior for large count values and SIZE_MAX cases.
>
>Is there a way to instead to fix the strnlen_zbb to avoid the fallback?
>

It is possible to make strnlen_zbb work correctly without fallback, I will reconsider this point:  if the performance loss 
of the non-fallback version turns out to be small, then using the non-fallback code may be acceptable.

Regards,  
Rui

Re: [PATCH] riscv: lib: Fix address overflow due to large count values in strnlen ZBB path

Posted by Jason Montleon 4 days, 21 hours ago

On Mon, Aug 24, 2026 at 4:12 AM <gao.rui@zte.com.cn> wrote:
>
>
> Hi Aurelien,
>
> >On 2026-08-19 16:18, gao.rui@zte.com.cn wrote:
> >> Hi all,
> >>
> >> This patch fixes an issue in the RISC-V ZBB optimized strnlen implementation.
> >>
> >> Problem description: When using the ZBB optimized strnlen implementation, passing very large count values (such as SIZE_MAX) can cause address overflow and return incorrect results.
> >>
> >> This issue was observed in device-mapper tests:
> >>
> >> sh-5.2# dmsetup create testname9 --table "0 8 zero"
> >> sh-5.2# cat /sys/block/dm-*/dm/name
> >> testname
> >> sh-5.2# dmsetup remove testname9
> >>
> >> The overflow in strnlen caused failures in string handling during device-mapper operations.
> >>
> >> Patch summary:
> >> - Explicitly introduce the strnlen_generic label.
> >> - Simplify the generic implementation loop.
> >
> >This part should be in a separate patch, separated from the bug fix, and
> >if possible with some benchmark.
>
> I agree, I will split the generic implementation changes into a separate patch and provide benchmark results
> to show the performance impact.

Hi Rui,
Have you made any progress on an updated patch? This problem appears
to be breaking LVM on recent kernel versions. We attempted to boot an
early riscv64 Fedora 45 image with kernel 7.2.7 under QEMU today and
booting failed unexpectedly.

After some digging we found that LVM was broken because the UUID for
dm-0 was truncated. Since we were using Libvirt/QEMU I adjusted the VM
configuration to disable ZBB after which the VM booted successfully.
```
 <cpu mode='custom' match='exact' check='none'>
    <model fallback='forbid'>rv64</model>
    <feature policy='disable' name='zbb'/>
  </cpu>
```

Reproducer: https://gist.github.com/jmontleon/4cca07e6e5e5aa06b6b3e98ced0524e1

Thank you,
Jason Montleon


>
> >> - Add fallback logic in strnlen_zbb to redirect to the generic path when a0 + a1 overflows.
> >> This ensures correct behavior for large count values and SIZE_MAX cases.
> >
> >Is there a way to instead to fix the strnlen_zbb to avoid the fallback?
> >
>
> It is possible to make strnlen_zbb work correctly without fallback, I will reconsider this point:  if the performance loss
> of the non-fallback version turns out to be small, then using the non-fallback code may be acceptable.
>
> Regards,
> Rui
>
>
>
>