Hi,

Here is the 5th version of patches to fix vsnprintf().

 - Fix to limit the size of width and precision.
 - Warn if the return size is over INT_MAX.

Previous version is here;

https://lore.kernel.org/all/177440550682.147866.1854734911195480940.stgit@devnote2/

In this version, negative precision is treated as zero to match the
previous behavior and check the field/precision passed as string
literals too[1/2]. Also, update bstr_printf() not to return negative
value[2/2].

Thank you,

---

Masami Hiramatsu (Google) (2):
      lib/vsprintf: Fix to check field_width and precision
      lib/vsprintf: Limit the returning size to INT_MAX


 lib/vsprintf.c |   54 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

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

On Wed, 25 Mar 2026 22:27:31 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here is the 5th version of patches to fix vsnprintf().
> 
>  - Fix to limit the size of width and precision.
>  - Warn if the return size is over INT_MAX.
> 
> Previous version is here;
> 
> https://lore.kernel.org/all/177440550682.147866.1854734911195480940.stgit@devnote2/
> 
> In this version, negative precision is treated as zero to match the
> previous behavior and check the field/precision passed as string
> literals too[1/2]. Also, update bstr_printf() not to return negative
> value[2/2].
> 

BTW, skip_atoi() is used for converting precision and width,
but this does not check the overflow. This is expected to be
checked by compiler (-Wformat-overflow) but it checks the
width <= INT_MAX, but precision <= LONG_MAX (why?) and clang
does not check precision.

To avoid this issue, below fix is needed, but I'm not sure
this is meaningful check, because with [1/2] change, the
return value is limited anyway, and it's easy to check
during the review process if an obviously abnormal
precision value is passed in the format string.

Thanks,

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 69dec9b18428..8846d3a960dc 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -187,10 +187,20 @@ static inline int skip_atoi(const char **s)
 	int i = 0;
 
 	do {
-		i = i*10 + *((*s)++) - '0';
+		int next = *((*s)++) - '0';
+		if (unlikely(i > INT_MAX / 10U ||
+			     (i == INT_MAX / 10U && next > INT_MAX % 10U))) {
+			goto overflow;
+		}
+		i = i*10 + next;
 	} while (isdigit(**s));
 
 	return i;
+
+overflow:
+	while (isdigit(**s))
+		(*s)++;
+	return INT_MAX;
 }
 
 /*



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

On Wed, Mar 25, 2026 at 10:41:58PM +0900, Masami Hiramatsu wrote:
> On Wed, 25 Mar 2026 22:27:31 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > Hi,
> > 
> > Here is the 5th version of patches to fix vsnprintf().
> > 
> >  - Fix to limit the size of width and precision.
> >  - Warn if the return size is over INT_MAX.
> > 
> > Previous version is here;
> > 
> > https://lore.kernel.org/all/177440550682.147866.1854734911195480940.stgit@devnote2/
> > 
> > In this version, negative precision is treated as zero to match the
> > previous behavior and check the field/precision passed as string
> > literals too[1/2]. Also, update bstr_printf() not to return negative
> > value[2/2].

> BTW, skip_atoi() is used for converting precision and width,
> but this does not check the overflow. This is expected to be
> checked by compiler (-Wformat-overflow) but it checks the
> width <= INT_MAX, but precision <= LONG_MAX (why?) and clang
> does not check precision.
> 
> To avoid this issue, below fix is needed, but I'm not sure
> this is meaningful check, because with [1/2] change, the
> return value is limited anyway, and it's easy to check
> during the review process if an obviously abnormal
> precision value is passed in the format string.

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c

I you event want to do that, it should use macros from overflow.h,
also see how kstrto*() and memparse() perform such checks. Also
this may slow down the conversion.

-- 
With Best Regards,
Andy Shevchenko

On Thu, 26 Mar 2026 11:54:27 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Wed, Mar 25, 2026 at 10:41:58PM +0900, Masami Hiramatsu wrote:
> > On Wed, 25 Mar 2026 22:27:31 +0900
> > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > 
> > > Hi,
> > > 
> > > Here is the 5th version of patches to fix vsnprintf().
> > > 
> > >  - Fix to limit the size of width and precision.
> > >  - Warn if the return size is over INT_MAX.
> > > 
> > > Previous version is here;
> > > 
> > > https://lore.kernel.org/all/177440550682.147866.1854734911195480940.stgit@devnote2/
> > > 
> > > In this version, negative precision is treated as zero to match the
> > > previous behavior and check the field/precision passed as string
> > > literals too[1/2]. Also, update bstr_printf() not to return negative
> > > value[2/2].
> 
> > BTW, skip_atoi() is used for converting precision and width,
> > but this does not check the overflow. This is expected to be
> > checked by compiler (-Wformat-overflow) but it checks the
> > width <= INT_MAX, but precision <= LONG_MAX (why?) and clang
> > does not check precision.
> > 
> > To avoid this issue, below fix is needed, but I'm not sure
> > this is meaningful check, because with [1/2] change, the
> > return value is limited anyway, and it's easy to check
> > during the review process if an obviously abnormal
> > precision value is passed in the format string.
> 
> > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> 
> I you event want to do that, it should use macros from overflow.h,
> also see how kstrto*() and memparse() perform such checks. Also
> this may slow down the conversion.

Agreed, I don't want to push it. Since this overflow currently
only happens on precision and only by string literals, I think
it is better to be checked by review process.

Thank you,

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 


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