[v4] lib/vsprintf: Fixes size check

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

Posted by Masami Hiramatsu (Google) 1 week, 2 days ago

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.

Fixes: 938df695e98d ("vsprintf: associate the format state with the format pointer")
Reported-by: David Laight <david.laight.linux@gmail.com>
Closes: https://lore.kernel.org/all/20260318151250.40fef0ab@pumpkin/
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 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);
 }
 
 /*

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

Posted by Masami Hiramatsu (Google) 1 week, 1 day ago

On Wed, 25 Mar 2026 11:25:16 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> 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.
> 

Hmm, I also found that width/precision passed as string literals
also missed the range check.

Thanks,

> Fixes: 938df695e98d ("vsprintf: associate the format state with the format pointer")
> Reported-by: David Laight <david.laight.linux@gmail.com>
> Closes: https://lore.kernel.org/all/20260318151250.40fef0ab@pumpkin/
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  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);
>  }
>  
>  /*
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

Posted by Petr Mladek 1 week, 1 day ago

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

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

Posted by Masami Hiramatsu (Google) 1 week, 1 day ago

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.
> 	 *
> 	 * These test cases document the current behaviour; should
> 	 * anyone ever feel the need to follow the standards more
> 	 * closely, this can be revisited.
> 	 */

Yeah, I also found this comment. So v5 drops the negative precision
support.


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

Of course no.

> Would it break any existing kernel caller?

it is possible. Anyway to update the behavior, we also need to update
the test case.

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

OK.

Thanks,

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


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

Posted by David Laight 1 week, 1 day ago

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

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

Posted by David Laight 1 week, 1 day ago

On Wed, 25 Mar 2026 11:29:22 +0000
David Laight <david.laight.linux@gmail.com> wrote:

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

The only format string matches for '".*%[-+ #0]*[0-9]*\.[a-z].*"' are in
printf_kuint.c
There are some "%*.s" lurking, most are outputting "" or " " for alignment,
the '.' can/should be removed, but truncating " " to "" makes no difference.
(Well, it might change one pad space to none...)
That leaves three "%*.s" in diagnostic printk() in dx_show_leaf() in
fs/ext4/namei.c - all should be "%.*s" anyway.
So "%.s" can safely be changed to be the same as "%.0s".

Changing "%.d" from being "%d" to "%.0d" only affects the conversion of zero.
But I didn't find any.

It is harder to check the ("%.*s" len, str) cases for a possible negative len.
Only really because of the shear number, most are 'namelen, name'.
I guess a script/program to convert ("%.*s", prec, ptr) to ("%.*s", FMT_PREC(prec), ptr)
then get the compiler to error !statically_true(prec >= 0) and look at
what it finds.
That should reduce the 700+ cases to a manageable number.

	David


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

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

Posted by David Laight 1 week, 1 day ago

On Wed, 25 Mar 2026 11:25:16 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> 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.
> 
> Fixes: 938df695e98d ("vsprintf: associate the format state with the format pointer")
> Reported-by: David Laight <david.laight.linux@gmail.com>
> Closes: https://lore.kernel.org/all/20260318151250.40fef0ab@pumpkin/
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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@gmail.com>

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);
>  }
>  
>  /*
>

[PATCH v4 1/2] lib/vsprintf: Fix to check field_width and precision
[PATCH v4 2/2] lib/vsprintf: Limit the returning size to INT_MAX