public inbox for linux-efi@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/1] lib/vsnprintf: Limit the returning size to INT_MAX
@ 2026-03-18  1:19 Masami Hiramatsu (Google)
  2026-03-18  1:19 ` [RFC PATCH 1/1] lib/vsprintf: " Masami Hiramatsu (Google)
  0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-03-18  1:19 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas
  Cc: Steven Rostedt, Josh Law, Andrew Morton, Masami Hiramatsu,
	linux-efi, linux-kernel

Hi,

Recently we discussed snprintf() usage in bootconfig and found snprintf()
design has a problem[1]. It returns the required or printed size in 'int'
but the maxlen is passed by 'size_t'. If the maxlen is bigger than INT_MAX
(this should never happen), it can not return the size correctly, that
becomes negative value or very shorter than it has actually done.

I think it should not be handled by caller side, instead, vsnprintf()
should handle it. This is an extremely unlikely input, and if we need to
address it, I think we should keep the fix to a minimum (in performance
point of view).

Thus I just limited the returning size to INT_MAX. If caller sees the
*printf() returns INT_MAX, it can handle it as an error or use strlen()
to get real printed size (but I don't recommend it.)

IMHO, this input is basically impossible unless done intentionally,
so I think it's a form of over-engineering. Therefore, this is an
RFC patch.

A question is that we should use WARN_ON() when the return size is over
INT_MAX. Currently this just returns INT_MAX (no warning) because this is
a library function, which can be called from anywhere.

[1] https://lore.kernel.org/all/20260317121507.30735331@gandalf.local.home/

Thank you,

---

Masami Hiramatsu (Google) (1):
      lib/vsprintf: Limit the returning size to INT_MAX


 drivers/firmware/efi/libstub/vsprintf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH 1/1] lib/vsprintf: Limit the returning size to INT_MAX
  2026-03-18  1:19 [RFC PATCH 0/1] lib/vsnprintf: Limit the returning size to INT_MAX Masami Hiramatsu (Google)
@ 2026-03-18  1:19 ` Masami Hiramatsu (Google)
  2026-03-18 13:47   ` Steven Rostedt
  2026-03-18 23:50   ` Masami Hiramatsu
  0 siblings, 2 replies; 6+ messages in thread
From: Masami Hiramatsu (Google) @ 2026-03-18  1:19 UTC (permalink / raw)
  To: Ard Biesheuvel, Ilias Apalodimas
  Cc: Steven Rostedt, Josh Law, Andrew Morton, Masami Hiramatsu,
	linux-efi, linux-kernel

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

There seems a design flaw of vsnprintf() whose return value can
overflow the INT_MAX even on 32bit arch, because the buffer size is
passed by 'size_t' but it returns the printed or required size in 'int'.

The size_t is unsigned long, thus the caller can pass bigger than INT_MAX
as the size of buffer (that is OK). But even the vsnprintf calculates
the required/printed length correctly, if it overflows the INT_MAX,
it can not return the size correctly by int.

This should never happen but it should be checked and limited.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 drivers/firmware/efi/libstub/vsprintf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
index 71c71c222346..1713cacecc25 100644
--- a/drivers/firmware/efi/libstub/vsprintf.c
+++ b/drivers/firmware/efi/libstub/vsprintf.c
@@ -549,7 +549,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
 	if (size)
 		buf[min(pos, size-1)] = '\0';
 
-	return pos;
+	return (pos > INT_MAX) ? INT_MAX : pos;
 }
 
 int snprintf(char *buf, size_t size, const char *fmt, ...)


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/1] lib/vsprintf: Limit the returning size to INT_MAX
  2026-03-18  1:19 ` [RFC PATCH 1/1] lib/vsprintf: " Masami Hiramatsu (Google)
@ 2026-03-18 13:47   ` Steven Rostedt
  2026-03-18 15:12     ` David Laight
  2026-03-18 23:50   ` Masami Hiramatsu
  1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2026-03-18 13:47 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Ard Biesheuvel, Ilias Apalodimas, Josh Law, Andrew Morton,
	linux-efi, linux-kernel

On Wed, 18 Mar 2026 10:19:56 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> There seems a design flaw of vsnprintf() whose return value can
> overflow the INT_MAX even on 32bit arch, because the buffer size is
> passed by 'size_t' but it returns the printed or required size in 'int'.
> 
> The size_t is unsigned long, thus the caller can pass bigger than INT_MAX
> as the size of buffer (that is OK). But even the vsnprintf calculates
> the required/printed length correctly, if it overflows the INT_MAX,
> it can not return the size correctly by int.
> 
> This should never happen but it should be checked and limited.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  drivers/firmware/efi/libstub/vsprintf.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
> index 71c71c222346..1713cacecc25 100644
> --- a/drivers/firmware/efi/libstub/vsprintf.c
> +++ b/drivers/firmware/efi/libstub/vsprintf.c
> @@ -549,7 +549,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
>  	if (size)
>  		buf[min(pos, size-1)] = '\0';
>  
> -	return pos;
> +	return (pos > INT_MAX) ? INT_MAX : pos;
>  }
>  
>  int snprintf(char *buf, size_t size, const char *fmt, ...)

Since this would require a buffer of size greater than 2G to be passed in,
I highly doubt this would happen anywhere in the kernel.

If anything, it would be for "correctness" only, but I don't see this ever
being an issue within this century.

-- Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/1] lib/vsprintf: Limit the returning size to INT_MAX
  2026-03-18 13:47   ` Steven Rostedt
@ 2026-03-18 15:12     ` David Laight
  2026-03-19  0:07       ` Masami Hiramatsu
  0 siblings, 1 reply; 6+ messages in thread
From: David Laight @ 2026-03-18 15:12 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu (Google), Ard Biesheuvel, Ilias Apalodimas,
	Josh Law, Andrew Morton, linux-efi, linux-kernel

On Wed, 18 Mar 2026 09:47:33 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 18 Mar 2026 10:19:56 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > There seems a design flaw of vsnprintf() whose return value can
> > overflow the INT_MAX even on 32bit arch, because the buffer size is
> > passed by 'size_t' but it returns the printed or required size in 'int'.
> > 
> > The size_t is unsigned long, thus the caller can pass bigger than INT_MAX
> > as the size of buffer (that is OK). But even the vsnprintf calculates
> > the required/printed length correctly, if it overflows the INT_MAX,
> > it can not return the size correctly by int.
> > 
> > This should never happen but it should be checked and limited.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  drivers/firmware/efi/libstub/vsprintf.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
> > index 71c71c222346..1713cacecc25 100644
> > --- a/drivers/firmware/efi/libstub/vsprintf.c
> > +++ b/drivers/firmware/efi/libstub/vsprintf.c
> > @@ -549,7 +549,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
> >  	if (size)
> >  		buf[min(pos, size-1)] = '\0';
> >  
> > -	return pos;
> > +	return (pos > INT_MAX) ? INT_MAX : pos;
> >  }
> >  
> >  int snprintf(char *buf, size_t size, const char *fmt, ...)  
> 
> Since this would require a buffer of size greater than 2G to be passed in,
> I highly doubt this would happen anywhere in the kernel.
> 
> If anything, it would be for "correctness" only, but I don't see this ever
> being an issue within this century.

What about the return value (on 32bit) from:
	snprintf(NULL, 0, " %*s %*s ", INT_MAX, "", INT_MAX, "");

This is more of a problem for libc.

In reality there are easier ways to crash the kernel....

	David


> 
> -- Steve
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/1] lib/vsprintf: Limit the returning size to INT_MAX
  2026-03-18  1:19 ` [RFC PATCH 1/1] lib/vsprintf: " Masami Hiramatsu (Google)
  2026-03-18 13:47   ` Steven Rostedt
@ 2026-03-18 23:50   ` Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2026-03-18 23:50 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: Ard Biesheuvel, Ilias Apalodimas, Steven Rostedt, Josh Law,
	Andrew Morton, linux-efi, linux-kernel

On Wed, 18 Mar 2026 10:19:56 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> There seems a design flaw of vsnprintf() whose return value can
> overflow the INT_MAX even on 32bit arch, because the buffer size is
> passed by 'size_t' but it returns the printed or required size in 'int'.
> 
> The size_t is unsigned long, thus the caller can pass bigger than INT_MAX
> as the size of buffer (that is OK). But even the vsnprintf calculates
> the required/printed length correctly, if it overflows the INT_MAX,
> it can not return the size correctly by int.
> 
> This should never happen but it should be checked and limited.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  drivers/firmware/efi/libstub/vsprintf.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
> index 71c71c222346..1713cacecc25 100644
> --- a/drivers/firmware/efi/libstub/vsprintf.c
> +++ b/drivers/firmware/efi/libstub/vsprintf.c

Ooops, I saw a wrong code. Nevermind.

> @@ -549,7 +549,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
>  	if (size)
>  		buf[min(pos, size-1)] = '\0';
>  
> -	return pos;
> +	return (pos > INT_MAX) ? INT_MAX : pos;
>  }
>  
>  int snprintf(char *buf, size_t size, const char *fmt, ...)
> 


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH 1/1] lib/vsprintf: Limit the returning size to INT_MAX
  2026-03-18 15:12     ` David Laight
@ 2026-03-19  0:07       ` Masami Hiramatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2026-03-19  0:07 UTC (permalink / raw)
  To: David Laight
  Cc: Steven Rostedt, Masami Hiramatsu (Google), Ard Biesheuvel,
	Ilias Apalodimas, Josh Law, Andrew Morton, linux-efi,
	linux-kernel

On Wed, 18 Mar 2026 15:12:50 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Wed, 18 Mar 2026 09:47:33 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Wed, 18 Mar 2026 10:19:56 +0900
> > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > 
> > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > 
> > > There seems a design flaw of vsnprintf() whose return value can
> > > overflow the INT_MAX even on 32bit arch, because the buffer size is
> > > passed by 'size_t' but it returns the printed or required size in 'int'.
> > > 
> > > The size_t is unsigned long, thus the caller can pass bigger than INT_MAX
> > > as the size of buffer (that is OK). But even the vsnprintf calculates
> > > the required/printed length correctly, if it overflows the INT_MAX,
> > > it can not return the size correctly by int.
> > > 
> > > This should never happen but it should be checked and limited.
> > > 
> > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > > ---
> > >  drivers/firmware/efi/libstub/vsprintf.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
> > > index 71c71c222346..1713cacecc25 100644
> > > --- a/drivers/firmware/efi/libstub/vsprintf.c
> > > +++ b/drivers/firmware/efi/libstub/vsprintf.c
> > > @@ -549,7 +549,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
> > >  	if (size)
> > >  		buf[min(pos, size-1)] = '\0';
> > >  
> > > -	return pos;
> > > +	return (pos > INT_MAX) ? INT_MAX : pos;
> > >  }
> > >  
> > >  int snprintf(char *buf, size_t size, const char *fmt, ...)  
> > 
> > Since this would require a buffer of size greater than 2G to be passed in,
> > I highly doubt this would happen anywhere in the kernel.
> > 
> > If anything, it would be for "correctness" only, but I don't see this ever
> > being an issue within this century.
> 
> What about the return value (on 32bit) from:
> 	snprintf(NULL, 0, " %*s %*s ", INT_MAX, "", INT_MAX, "");
> 
> This is more of a problem for libc.
> 
> In reality there are easier ways to crash the kernel....

Good catch! I found that size >= INT_MAX has been handled correctly,
(see lib/vsprintf.c:2875) but width check has been removed by
commit 938df695e98d ("vsprintf: associate the format state with the
 format pointer") accidentally.

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 800b8ac49f53..054c9758118e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2803,7 +2803,8 @@ 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)) {
+	if (WARN_ONCE(spec->field_width > FIELD_WIDTH_MAX ||
+		      spec->field_width < -FIELD_WIDTH_MAX, "field width %d too large", width)) {
 		spec->field_width = clamp(width, -FIELD_WIDTH_MAX, FIELD_WIDTH_MAX);
 	}
 }
@@ -2812,7 +2813,8 @@ static void
 set_precision(struct printf_spec *spec, int prec)
 {
 	spec->precision = prec;
-	if (WARN_ONCE(spec->precision != prec, "precision %d too large", prec)) {
+	if (WARN_ONCE(spec->precision > PRECISION_MAX || spec->precision < 0,
+		      "precision %d too large", prec)) {
 		spec->precision = clamp(prec, 0, PRECISION_MAX);
 	}
 }

> 
> 	David
> 
> 
> > 
> > -- Steve
> > 
> 


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-19  0:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-18  1:19 [RFC PATCH 0/1] lib/vsnprintf: Limit the returning size to INT_MAX Masami Hiramatsu (Google)
2026-03-18  1:19 ` [RFC PATCH 1/1] lib/vsprintf: " Masami Hiramatsu (Google)
2026-03-18 13:47   ` Steven Rostedt
2026-03-18 15:12     ` David Laight
2026-03-19  0:07       ` Masami Hiramatsu
2026-03-18 23:50   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox