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

[David Laight <david.laight.linux@gmail.com>]

On Wed, 25 Mar 2026 11:22:47 +0100
Petr Mladek <pmladek@suse.com> wrote:

> 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.

Ugg...

	David

> 	 *
> 	 * 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