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

[Aurelien Jarno <aurelien@aurel32.net>]

On 2026-09-15 15:26, shao.mingyin@zte.com.cn wrote:
> 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
> clamps the increment to ~s, so it stays branchless and wrap-free:
> 
> 	s + min(count - 1, ~s) == saturating_add(s, count - 1)
> 
> 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>
> Suggested-by: Qingfang Deng <qingfang.deng@linux.dev>
> Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
> Acked-by: Michael Neuling <mikey@neuling.org>
> ---
> Changes in v4:
> - Use Zbb minu to clamp the increment (addi/not/minu/add): one
>   instruction less than the sltu/mask/or sequence and no extra
>   register (Qingfang Deng).  Clobbers stays t0-t4.
> 
> 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.
> 
> v3: https://lore.kernel.org/all/20260914162123230u1y1M4UHrO8E-cU-opJ_7@zte.com.cn/
> 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 | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

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