[PATCH v3] riscv: lib: Fix ZBB strnlen wrap-around regression on huge counts

[PATCH v3] riscv: lib: Fix ZBB strnlen wrap-around regression on huge counts

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

The aligned scan boundary is derived from the last valid byte,
(s + count - 1).  When count is huge (e.g. SIZE_MAX, which FORTIFY
strcat/strlcat pass when the destination size is not known at compile
time), s + count wraps around and the boundary lands before s, so the
ZBB path returns a bogus length.  The original implementation
(5ba15d419fab) had the same wrap-around in its (s + count) & ~7
boundary computation; after 5d588c684833 the wrapped boundary is caught
by the pre-loop guard "bgeu t0, t4, 2f", which then always exits for
aligned strings of 8 or more characters and strnlen() returns 8
instead of the real length.

This silently truncates strings built by fortified strcat: the dm
sysfs name attribute shows "live-bas" instead of "live-base", the
truncated name pollutes the udev database, and blivet/anaconda (as
well as LVM/dm-crypt/multipath userspace) break on RISC-V systems.

Detect the wrap-around and saturate the boundary to the top of the
address space, making the scan equivalent to strlen().  The saturation
is branchless (sltu/mask/or), so the normal path does not carry a taken
branch.  Normal counts are unaffected.

Fixes: 5ba15d419fab ("riscv: lib: add strnlen() implementation")
Cc: stable@vger.kernel.org
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
Acked-by: Michael Neuling <mikey@neuling.org>
---
Changes in v3:
- Replace the taken branch in the saturation with a branchless
  sltu/mask/or sequence (David Laight).
- Update the Clobbers list for the additional t5 register.
- Michael's Acked-by is kept: the patch semantics are unchanged, only
  the saturation sequence is branchless now.

Changes in v2:
- Point Fixes: at the original implementation (5ba15d419fab) and reword
  the commit message accordingly: the wrap-around exists since the
  original implementation, 5d588c684833 only changed how it surfaces
  (Michael Neuling).
- Add Acked-by from Michael Neuling.

v2: https://lore.kernel.org/all/20260914145205778-sZJbZc1D-XBfWRXO2f-o@zte.com.cn/
v1: https://lore.kernel.org/all/20260828145152578tXQPUG9lxxgbJjmfpuaQz@zte.com.cn/

 arch/riscv/lib/strnlen.S | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/lib/strnlen.S b/arch/riscv/lib/strnlen.S
index a8911605c248..528ee7995969 100644
--- a/arch/riscv/lib/strnlen.S
+++ b/arch/riscv/lib/strnlen.S
@@ -67,7 +67,7 @@ strnlen_zbb:
 	 *   a1 - Max length of string
 	 *
 	 * Clobbers
-	 *   t0, t1, t2, t3, t4
+	 *   t0, t1, t2, t3, t4, t5
 	 */

 	/* If maxlen is 0, return 0. */
@@ -87,9 +87,22 @@ strnlen_zbb:
 	 * Aligned boundary.  Use the address of the last valid byte
 	 * (s + count - 1) to avoid loading a word past the count
 	 * boundary in the loop below.  count == 0 is handled above.
+	 *
+	 * Saturate the boundary when s + count wraps around (very large
+	 * counts, e.g. SIZE_MAX passed by FORTIFY strcat/strlcat with a
+	 * destination whose size is unknown at compile time).  Without
+	 * this, the wrapped boundary lands before s and the pre-loop
+	 * guard below always exits, returning a truncated length.
+	 * Saturating makes the scan equivalent to strlen().
+	 *
+	 * Keep the saturation branchless so that the normal path does
+	 * not carry a taken branch that could be mispredicted.
 	 */
 	add	t4, a0, a1
 	addi	t4, t4, -1
+	sltu	t5, t4, a0	/* Did s + count wrap around? */
+	sub	t5, zero, t5	/* -1 if wrapped, 0 otherwise */
+	or	t4, t4, t5	/* Saturate to the top of the address space */
 	andi	t4, t4, -SZREG

 	/* Get the first word.  */
-- 
2.27.0

Re: [PATCH v3] riscv: lib: Fix ZBB strnlen wrap-around regression on huge counts

[Qingfang Deng]

Hi,

On 2026/9/14 16:21, shao.mingyin@zte.com.cn wrote:
> diff --git a/arch/riscv/lib/strnlen.S b/arch/riscv/lib/strnlen.S
> index a8911605c248..528ee7995969 100644
> --- a/arch/riscv/lib/strnlen.S
> +++ b/arch/riscv/lib/strnlen.S
> @@ -67,7 +67,7 @@ strnlen_zbb:
>   	 *   a1 - Max length of string
>   	 *
>   	 * Clobbers
> -	 *   t0, t1, t2, t3, t4
> +	 *   t0, t1, t2, t3, t4, t5
>   	 */
>
>   	/* If maxlen is 0, return 0. */
> @@ -87,9 +87,22 @@ strnlen_zbb:
>   	 * Aligned boundary.  Use the address of the last valid byte
>   	 * (s + count - 1) to avoid loading a word past the count
>   	 * boundary in the loop below.  count == 0 is handled above.
> +	 *
> +	 * Saturate the boundary when s + count wraps around (very large
> +	 * counts, e.g. SIZE_MAX passed by FORTIFY strcat/strlcat with a
> +	 * destination whose size is unknown at compile time).  Without
> +	 * this, the wrapped boundary lands before s and the pre-loop
> +	 * guard below always exits, returning a truncated length.
> +	 * Saturating makes the scan equivalent to strlen().
> +	 *
> +	 * Keep the saturation branchless so that the normal path does
> +	 * not carry a taken branch that could be mispredicted.
>   	 */
>   	add	t4, a0, a1
>   	addi	t4, t4, -1
> +	sltu	t5, t4, a0	/* Did s + count wrap around? */
> +	sub	t5, zero, t5	/* -1 if wrapped, 0 otherwise */
> +	or	t4, t4, t5	/* Saturate to the top of the address space */
You can save one instruction by using Zbb minu instruction.

Equation: saturating_add(s, count - 1) == s + min(count - 1, ~s), given 
that count >= 1.

     addi    t4, a1, -1
     not    t1, a0
     minu    t4, t4, t1
     add    t4, a0, t4
>   	andi	t4, t4, -SZREG
>
>   	/* Get the first word.  */
Kind regards,
Qingfang