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

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

[Thorsten Blum]

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>
---
 sound/core/info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/core/info.c b/sound/core/info.c
index 023b4736aa6d..b0ed66d0336d 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -484,7 +484,7 @@ int snd_info_card_create(struct snd_card *card)
 	if (snd_BUG_ON(!card))
 		return -ENXIO;
 
-	sprintf(str, "card%i", card->number);
+	snprintf(str, sizeof(str), "card%i", card->number);
 	entry = create_subdir(card->module, str);
 	if (!entry)
 		return -ENOMEM;

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

[Takashi Iwai]

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.


thanks,

Takashi

> ---
>  sound/core/info.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/core/info.c b/sound/core/info.c
> index 023b4736aa6d..b0ed66d0336d 100644
> --- a/sound/core/info.c
> +++ b/sound/core/info.c
> @@ -484,7 +484,7 @@ int snd_info_card_create(struct snd_card *card)
>  	if (snd_BUG_ON(!card))
>  		return -ENXIO;
>  
> -	sprintf(str, "card%i", card->number);
> +	snprintf(str, sizeof(str), "card%i", card->number);
>  	entry = create_subdir(card->module, str);
>  	if (!entry)
>  		return -ENOMEM;

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

[Thorsten Blum]

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.

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

[Takashi Iwai]

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.


Takashi

--- a/sound/core/ump.c
+++ b/sound/core/ump.c
@@ -1369,7 +1369,7 @@ static void update_legacy_substreams(struct snd_ump_endpoint *ump,
 		name = ump->groups[idx].name;
 		if (!*name)
 			name = ump->core.name;
-		scnprintf(s->name, sizeof(s->name), "Group %d (%.16s)%s",
+		snprintf(s->name, sizeof(s->name), "Group %d (%.16s)%s",
 			  idx + 1, name,
 			  ump->groups[idx].active ? "" : " [Inactive]");
 		s->inactive = !ump->groups[idx].active;

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

[Thorsten Blum]

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.

I don't see a warning when compiling my patch with W=1, but I'm happy to
switch to scnprintf() if you prefer.

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

[Takashi Iwai]

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().


thanks,

Takashi

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

[David Laight]

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
>