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

[Petr Mladek <pmladek@suse.com>]

On Wed 2026-03-25 11:25:16, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 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.
> 
> --- 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;

This changes the existing kernel behavior and breaks the existing
KUnit test in lib/tests/printf_kunit.c:

static void
test_string(struct kunit *kunittest)
{
[...]
	/*
	 * POSIX and C99 say that a negative precision (which is only
	 * possible to pass via a * argument) should be treated as if
	 * the precision wasn't present, and that if the precision is
	 * omitted (as in %.s), the precision should be taken to be
	 * 0. However, the kernel's printf behave exactly opposite,
	 * treating a negative precision as 0 and treating an omitted
	 * precision specifier as if no precision was given.
	 *
	 * These test cases document the current behaviour; should
	 * anyone ever feel the need to follow the standards more
	 * closely, this can be revisited.
	 */
	test("    ", "%4.*s", -5, "123456");
[...]
}

The output is:

[   86.234405]     # test_string: EXPECTATION FAILED at lib/tests/printf_kunit.c:56
               lib/tests/printf_kunit.c:208: vsnprintf(buf, 256, "%4.*s", ...) returned 6, expected 4
[   86.237524]     # test_string: EXPECTATION FAILED at lib/tests/printf_kunit.c:56
               lib/tests/printf_kunit.c:208: vsnprintf(buf, 2, "%4.*s", ...) returned 6, expected 4
[   86.237542]     # test_string: EXPECTATION FAILED at lib/tests/printf_kunit.c:56
               lib/tests/printf_kunit.c:208: vsnprintf(buf, 0, "%4.*s", ...) returned 6, expected 4
[   86.237559]     # test_string: EXPECTATION FAILED at lib/tests/printf_kunit.c:141
               lib/tests/printf_kunit.c:208: kvasprintf(..., "%4.*s", ...) returned '123456', expected '    '

Do we really want to change the existing behavior?
Would it break any existing kernel caller?

I would personally keep the existing behavior unless anyone checks
the existing callers.

> -
>  		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);

And I would keep clamp(prec, 0, PRECISION_MAX) unless anyone checks
that changing the existing behavior does not break existing
callers.

> +	WARN_ONCE(spec->precision < prec, "precision %d too large", prec);
>  }

Best Regards,
Petr