Re: [PATCH] ALSA: info: Use snprintf() in snd_info_card_create()

[David Laight <david.laight.linux@gmail.com>]

On Sat, 07 Nov 2026 15:02:58 +0100
Takashi Iwai <tiwai@suse.de> wrote:

> On Sat, 07 Nov 2026 14:57:09 +0100,
> Thorsten Blum wrote:
> > 
> > On Sat, Nov 07, 2026 at 02:44:57PM +0100, Takashi Iwai wrote:  
> > > On Sat, 07 Nov 2026 14:04:33 +0100,
> > > Thorsten Blum wrote:  
> > > > 
> > > > On Sat, Nov 07, 2026 at 10:14:38AM +0100, Takashi Iwai wrote:  
> > > > > On Sat, 07 Nov 2026 01:43:37 +0100,
> > > > > Thorsten Blum wrote:  
> > > > > > 
> > > > > > While the current code works correctly, replace the unbounded sprintf()
> > > > > > with snprintf() to bound the formatted string to the str buffer.
> > > > > > 
> > > > > > Signed-off-by: Thorsten Blum <blum@kernel.org>  
> > > > > 
> > > > > Shouldn't we use scnprintf() instead?  Otherwise it may lead to a
> > > > > compiler warning.  
> > > > 
> > > > The only difference between snprintf() and scnprintf() is the return
> > > > value, which is unused here. Both are correct and shouldn't trigger a
> > > > warning.  
> > > 
> > > Well, not really.  For this particular patch, it doesn't make
> > > difference, but in some cases (like the use with %s), snprintf()
> > > causes a spurious compile warning with W=1.  e.g. try below.  
> > 
> > Agreed, although that's a different use case from my patch.  
>  
> Yep.  I guess a warning is triggered only when filling a substring
> that can be large into the fixed size buffer for now, but it's a
> compiler's thing, that can change at any time in future...
> 
> > I don't see a warning when compiling my patch with W=1, but I'm happy to
> > switch to scnprintf() if you prefer.  
> 
> I'd like to bet for a safer way, so let's go with scnprintf().

At some point someone will work out why scnprintf() doesn't generate
the error.
OTOH there really ought to be a way of telling gcc that the overflow
is a false positive or you really don't care.
Doing some checks with the result stops the warning, but a (void)
cast doesn't - much like it doesn't stop a 'must_check' error.

The real this is it that it is somewhat pointless.
It basically assumes that any char[] might be its maximum length and
any integer its maximum value (%d needs 10 characters).
But it can't report for 'char *' because it doesn't know the length.
I can really imagine people breaking code in all sorts of ways while
trying to stop the warning.
In particular code might start trying to 'exactly size' strings instead
of using ones that are 'really far too big' (an on-stack char[4096]
really doesn't matter in userspace) and then getting actual overflows
(or memory leaks) for normal data.

David

> 
> 
> thanks,
> 
> Takashi
>