linux-kernel - Re: [PATCH] staging: greybus: audio: avoid snprintf truncation warnings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251229141730.2b863ba9@pumpkin>
Date: Mon, 29 Dec 2025 14:17:30 +0000
From: David Laight <david.laight.linux@...il.com>
To: Sun Jian <sun.jian.kdev@...il.com>
Cc: Vaibhav Agarwal <vaibhav.sr@...il.com>, Mark Greer
 <mgreer@...malcreek.com>, Alex Elder <elder@...nel.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, greybus-dev@...ts.linaro.org,
 linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: greybus: audio: avoid snprintf truncation
 warnings

On Mon, 29 Dec 2025 19:26:49 +0800
Sun Jian <sun.jian.kdev@...il.com> wrote:

> W=1 reports possible truncation when formatting widget and control names
> using snprintf() with a %s argument and fixed-size buffers. Build the
> prefixed names using scnprintf() plus strlcat() instead, so truncation,
> if any, is handled by the string helpers rather than during printf
> formatting.

That is just brain dead - more so than the fact the the warning is pretty
hard to ignore.
Commonning up the code as:
static void prefix_div_id(char *name, size_t name_len, unsigned int dev_id)
{
	char temp_name[32], *cp = temp_name;

	strscpy(temp_name, name, sizeof temp_name);
	OPTIMISER_HIDE_VAR(cp);
	snprintf(name, name_len, "GB %d %s", dev_id, cp);
}
should be enough and is about the best you can do.
Possibly worth a comment that name_len is expected to be 32.

The way this code processes the tplg_data isn't nice at all.
Convert the offsets to correctly typed pointers as soon as possible.
You might want to verify that the separate arrays don't overlap.

There is also the:
	curr = (void *)curr + w_size;
which AFIACT is just 'curr++';

	David

> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@...il.com>
> ---
>  drivers/staging/greybus/audio_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> index 76146f91cddc..4293ab899390 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1087,7 +1087,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  
>  	/* Prefix dev_id to widget control_name */
>  	strscpy(temp_name, w->name, sizeof(temp_name));
> -	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
> +	scnprintf(w->name, sizeof(w->name), "GB %d ", module->dev_id);
> +	strlcat(w->name, temp_name, sizeof(w->name));
>  
>  	switch (w->type) {
>  	case snd_soc_dapm_spk:
> @@ -1169,8 +1170,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
>  		control->id = curr->id;
>  		/* Prefix dev_id to widget_name */
>  		strscpy(temp_name, curr->name, sizeof(temp_name));
> -		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
> -			 temp_name);
> +		scnprintf(curr->name, sizeof(curr->name), "GB %d ", module->dev_id);
> +		strlcat(curr->name, temp_name, sizeof(curr->name));
>  		control->name = curr->name;
>  		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
>  			struct gb_audio_enumerated *gbenum =