1. Patchew
  2. linux
  3. View series

[PATCH v4 0/2] lib/vsprintf: Fixes size check

Masami Hiramatsu (Google) posted 2 patches 1 week, 1 day ago
There is a newer version of this series
lib/vsprintf.c |   24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
[PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by Masami Hiramatsu (Google) 1 week, 1 day ago 
Hi,

Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/

In this version, do clamp() the width and precision before checking it and
accept negative precision[1/3] and add Petr's Reviewed-by[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 |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by Andrew Morton 1 week, 1 day ago 
On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> 
> In this version, do clamp() the width and precision before checking it and
> accept negative precision[1/3] and add Petr's Reviewed-by[2/2].

AI review has flagged a couple of possible issues:
	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by David Laight 1 week, 1 day ago 
On Tue, 24 Mar 2026 22:04:58 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > 
> > In this version, do clamp() the width and precision before checking it and
> > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].  
> 
> AI review has flagged a couple of possible issues:
> 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2

I'd guess there are exactly 0 places where a negative precision is passed
to "%.*s" - if there were any someone would have complained about the
output being missing.
Checking all 759 cases grep -r '".*%.*\.%*s.*"' found will be tedious.
But pretty much all are 'namelen'.

In any case worst thing should be a panic if the code hits an invalid
address before finding a '\0' byte - probably unlikely anyway.

I'd fix it, but try to stop it being backported.

	David
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by Masami Hiramatsu (Google) 1 week ago 
On Wed, 25 Mar 2026 10:20:39 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Tue, 24 Mar 2026 22:04:58 -0700
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > 
> > > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > > 
> > > In this version, do clamp() the width and precision before checking it and
> > > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].  
> > 
> > AI review has flagged a couple of possible issues:
> > 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2
> 
> I'd guess there are exactly 0 places where a negative precision is passed
> to "%.*s" - if there were any someone would have complained about the
> output being missing.
> Checking all 759 cases grep -r '".*%.*\.%*s.*"' found will be tedious.
> But pretty much all are 'namelen'.

I also verified and found only one suspicious usage which can pass
a negative precision.

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index d2c7b1090df0..1f90775ea8a8 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -2224,7 +2224,7 @@ void i915_request_show(struct drm_printer *m,
 	rcu_read_lock();
 	timeline = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
 	drm_printf(m, "%s%.*s%c %llx:%lld%s%s %s @ %dms: %s\n",
-		   prefix, indent, "                ",
+		   prefix, max(0, indent), "                ",
 		   queue_status(rq),
 		   rq->fence.context, rq->fence.seqno,
 		   run_status(rq),

Thanks,
> 
> In any case worst thing should be a panic if the code hits an invalid
> address before finding a '\0' byte - probably unlikely anyway.
> 
> I'd fix it, but try to stop it being backported.
> 
> 	David


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by David Laight 1 week ago 
On Thu, 26 Mar 2026 16:39:44 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Wed, 25 Mar 2026 10:20:39 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
> 
> > On Tue, 24 Mar 2026 22:04:58 -0700
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> >   
> > > On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > >   
> > > > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > > > 
> > > > In this version, do clamp() the width and precision before checking it and
> > > > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].    
> > > 
> > > AI review has flagged a couple of possible issues:
> > > 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2  
> > 
> > I'd guess there are exactly 0 places where a negative precision is passed
> > to "%.*s" - if there were any someone would have complained about the
> > output being missing.
> > Checking all 759 cases grep -r '".*%.*\.%*s.*"' found will be tedious.
> > But pretty much all are 'namelen'.  
> 
> I also verified and found only one suspicious usage which can pass
> a negative precision.

It is always called with a constant, in any case the string being output
is constant so nothing nasty can happen.
I didn't even see any recursive/loop calls that indent by significant amounts.
The code could use the more usual ("%*s", indent, ""), but it doesn't matter
much - mostly just a shorter line.

	David

> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index d2c7b1090df0..1f90775ea8a8 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -2224,7 +2224,7 @@ void i915_request_show(struct drm_printer *m,
>  	rcu_read_lock();
>  	timeline = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
>  	drm_printf(m, "%s%.*s%c %llx:%lld%s%s %s @ %dms: %s\n",
> -		   prefix, indent, "                ",
> +		   prefix, max(0, indent), "                ",
>  		   queue_status(rq),
>  		   rq->fence.context, rq->fence.seqno,
>  		   run_status(rq),
> 
> Thanks,
> > 
> > In any case worst thing should be a panic if the code hits an invalid
> > address before finding a '\0' byte - probably unlikely anyway.
> > 
> > I'd fix it, but try to stop it being backported.
> > 
> > 	David  
> 
>
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by Masami Hiramatsu (Google) 6 days, 17 hours ago 
On Thu, 26 Mar 2026 09:12:12 +0000
David Laight <david.laight.linux@gmail.com> wrote:

> On Thu, 26 Mar 2026 16:39:44 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > On Wed, 25 Mar 2026 10:20:39 +0000
> > David Laight <david.laight.linux@gmail.com> wrote:
> > 
> > > On Tue, 24 Mar 2026 22:04:58 -0700
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > >   
> > > > On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > > >   
> > > > > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > > > > 
> > > > > In this version, do clamp() the width and precision before checking it and
> > > > > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].    
> > > > 
> > > > AI review has flagged a couple of possible issues:
> > > > 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2  
> > > 
> > > I'd guess there are exactly 0 places where a negative precision is passed
> > > to "%.*s" - if there were any someone would have complained about the
> > > output being missing.
> > > Checking all 759 cases grep -r '".*%.*\.%*s.*"' found will be tedious.
> > > But pretty much all are 'namelen'.  
> > 
> > I also verified and found only one suspicious usage which can pass
> > a negative precision.
> 
> It is always called with a constant, in any case the string being output
> is constant so nothing nasty can happen.

Since this function is not a static function, it can be called anywhere in
the module. So it is safer to check the indent is positive.

> I didn't even see any recursive/loop calls that indent by significant amounts.
> The code could use the more usual ("%*s", indent, ""), but it doesn't matter
> much - mostly just a shorter line.

I think the current code looks a bit tricky and not clear what it does.
Maybe it is better to replace it with above usual usage.

Thanks,

> 
> 	David
> 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > index d2c7b1090df0..1f90775ea8a8 100644
> > --- a/drivers/gpu/drm/i915/i915_request.c
> > +++ b/drivers/gpu/drm/i915/i915_request.c
> > @@ -2224,7 +2224,7 @@ void i915_request_show(struct drm_printer *m,
> >  	rcu_read_lock();
> >  	timeline = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
> >  	drm_printf(m, "%s%.*s%c %llx:%lld%s%s %s @ %dms: %s\n",
> > -		   prefix, indent, "                ",
> > +		   prefix, max(0, indent), "                ",
> >  		   queue_status(rq),
> >  		   rq->fence.context, rq->fence.seqno,
> >  		   run_status(rq),
> > 
> > Thanks,
> > > 
> > > In any case worst thing should be a panic if the code hits an invalid
> > > address before finding a '\0' byte - probably unlikely anyway.
> > > 
> > > I'd fix it, but try to stop it being backported.
> > > 
> > > 	David  
> > 
> > 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by David Laight 6 days, 15 hours ago 
On Fri, 27 Mar 2026 16:28:12 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Thu, 26 Mar 2026 09:12:12 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
> 
> > On Thu, 26 Mar 2026 16:39:44 +0900
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> >   
> > > On Wed, 25 Mar 2026 10:20:39 +0000
> > > David Laight <david.laight.linux@gmail.com> wrote:
> > >   
> > > > On Tue, 24 Mar 2026 22:04:58 -0700
> > > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > >     
> > > > > On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> > > > >     
> > > > > > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > > > > > 
> > > > > > In this version, do clamp() the width and precision before checking it and
> > > > > > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].      
> > > > > 
> > > > > AI review has flagged a couple of possible issues:
> > > > > 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2    
> > > > 
> > > > I'd guess there are exactly 0 places where a negative precision is passed
> > > > to "%.*s" - if there were any someone would have complained about the
> > > > output being missing.
> > > > Checking all 759 cases grep -r '".*%.*\.%*s.*"' found will be tedious.
> > > > But pretty much all are 'namelen'.    
> > > 
> > > I also verified and found only one suspicious usage which can pass
> > > a negative precision.  
> > 
> > It is always called with a constant, in any case the string being output
> > is constant so nothing nasty can happen.  
> 
> Since this function is not a static function, it can be called anywhere in
> the module. So it is safer to check the indent is positive.

That would have to be an out or tree module using a function that looks
pretty specific to the i915 display code.
Even as someone who has supported out of tree drivers I'd say that they
get what they deserve.
The snprintf() code itself won't break anything.

	David
 
> 
> > I didn't even see any recursive/loop calls that indent by significant amounts.
> > The code could use the more usual ("%*s", indent, ""), but it doesn't matter
> > much - mostly just a shorter line.  
> 
> I think the current code looks a bit tricky and not clear what it does.
> Maybe it is better to replace it with above usual usage.
> 
> Thanks,
> 
> > 
> > 	David
> >   
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> > > index d2c7b1090df0..1f90775ea8a8 100644
> > > --- a/drivers/gpu/drm/i915/i915_request.c
> > > +++ b/drivers/gpu/drm/i915/i915_request.c
> > > @@ -2224,7 +2224,7 @@ void i915_request_show(struct drm_printer *m,
> > >  	rcu_read_lock();
> > >  	timeline = dma_fence_timeline_name((struct dma_fence *)&rq->fence);
> > >  	drm_printf(m, "%s%.*s%c %llx:%lld%s%s %s @ %dms: %s\n",
> > > -		   prefix, indent, "                ",
> > > +		   prefix, max(0, indent), "                ",
> > >  		   queue_status(rq),
> > >  		   rq->fence.context, rq->fence.seqno,
> > >  		   run_status(rq),
> > > 
> > > Thanks,  
> > > > 
> > > > In any case worst thing should be a panic if the code hits an invalid
> > > > address before finding a '\0' byte - probably unlikely anyway.
> > > > 
> > > > I'd fix it, but try to stop it being backported.
> > > > 
> > > > 	David    
> > > 
> > >   
> >   
> 
>
Re: [PATCH v4 0/2] lib/vsprintf: Fixes size check
Posted by Masami Hiramatsu (Google) 1 week, 1 day ago 
On Tue, 24 Mar 2026 22:04:58 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > Here is the 4th 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/177410406326.38798.16853803119128725972.stgit@devnote2/
> > 
> > In this version, do clamp() the width and precision before checking it and
> > accept negative precision[1/3] and add Petr's Reviewed-by[2/2].
> 
> AI review has flagged a couple of possible issues:
> 	https://sashiko.dev/#/patchset/177440550682.147866.1854734911195480940.stgit@devnote2

OK, I think I should decouple accept negative precision part from [1/2] since
it is changing the kernel specific behavior.

Thanks, 

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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.