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

[David Laight]

On Thu, 26 Mar 2026 16:39:44 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:

> On Wed, 25 Mar 2026 10:20:39 +0000
> David Laight <david.laight.linux@xxxxxxxxx> wrote:
> 
> > On Tue, 24 Mar 2026 22:04:58 -0700
> > Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >   
> > > On Wed, 25 Mar 2026 11:25:06 +0900 "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> 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  
> 
>