Re: [PATCH v4 1/2] lib/vsprintf: Fix to check field_width and precision

[David Laight]

On Wed, 25 Mar 2026 11:25:16 +0900
"Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> 
> Check the field_width and presition correctly. Previously it depends
> on the bitfield conversion from int to check out-of-range error.
> However, commit 938df695e98d ("vsprintf: associate the format state
> with the format pointer") changed those fields to int.
> We need to check the out-of-range correctly without bitfield
> conversion.
> 
> Fixes: 938df695e98d ("vsprintf: associate the format state with the format pointer")
> Reported-by: David Laight <david.laight.linux@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/20260318151250.40fef0ab@pumpkin/
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

I've just read the code - my god is it complicated and confusing.
But I think it looks ok, so:
Reviewed-by: David Laight <david.laight.linux@xxxxxxxxx>

Maybe I'll find to time (haha) to rewrite it based on the nolibc version.
It'll be a lot smaller and a lot faster.
I also suspect that corner cases like ("%#.0o", 0) (where the '#' needs to
add a leading zero to the empty string) aren't right.
But that is different from this fix.

	David


> ---
>  Changes in v4:
>   - Do clamp() first.
>   - Accept negative precision (this means no precision) .
>   - Change the warning message for width.
>  Changes in v3:
>   - Check and update width and precision before assigning to spec.
>  Changes in v2:
>   - Fix to use logical split.
> ---
>  lib/vsprintf.c |   17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 800b8ac49f53..5fa8f69030be 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2679,9 +2679,6 @@ struct fmt format_decode(struct fmt fmt, struct printf_spec *spec)
>  
>  	/* we finished early by reading the precision */
>  	if (unlikely(fmt.state == FORMAT_STATE_PRECISION)) {
> -		if (spec->precision < 0)
> -			spec->precision = 0;
> -
>  		fmt.state = FORMAT_STATE_NONE;
>  		goto qualifier;
>  	}
> @@ -2802,19 +2799,17 @@ struct fmt format_decode(struct fmt fmt, struct printf_spec *spec)
>  static void
>  set_field_width(struct printf_spec *spec, int width)
>  {
> -	spec->field_width = width;
> -	if (WARN_ONCE(spec->field_width != width, "field width %d too large", width)) {
> -		spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
> -	}
> +	spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
> +	WARN_ONCE(spec->field_width != width, "field width %d out of range",
> +		  width);
>  }
>  
>  static void
>  set_precision(struct printf_spec *spec, int prec)
>  {
> -	spec->precision = prec;
> -	if (WARN_ONCE(spec->precision != prec, "precision %d too large", prec)) {
> -		spec->precision = clamp(prec, 0, PRECISION_MAX);
> -	}
> +	/* We allow negative precision, but treat it as if there was no precision. */
> +	spec->precision = clamp(prec, -1, PRECISION_MAX);
> +	WARN_ONCE(spec->precision < prec, "precision %d too large", prec);
>  }
>  
>  /*
>