linux-kernel - Re: [PATCH v3] 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <2025123028-transform-nutshell-81fa@gregkh>
Date: Tue, 30 Dec 2025 08:40:48 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Sun Jian <sun.jian.kdev@...il.com>
Cc: Vaibhav Agarwal <vaibhav.sr@...il.com>, Johan Hovold <johan@...nel.org>,
	Mark Greer <mgreer@...malcreek.com>, Alex Elder <elder@...nel.org>,
	David Laight <david.laight.linux@...il.com>,
	greybus-dev@...ts.linaro.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] staging: greybus: audio: avoid snprintf truncation
 warnings

On Tue, Dec 30, 2025 at 09:29:08AM +0800, Sun Jian wrote:
> W=1 reports possible truncation when formatting widget/control names
> with snprintf() and a %s argument. Use a small helper and hide the %s
> pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
> while keeping the existing snprintf() formatting.
> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@...il.com>
> 
> Changes in v3:
> - Replace the earlier scnprintf()/strlcat() approach with a helper
>   keeping snprintf().
> - Hide the %s argument from compiler truncation analysis using
>   OPTIMIZER_HIDE_VAR().
> - Add a small local length limit macro with a short comment.
> ---

The "changes" go below the --- line, as the documentation asks for.  And
please include what changed from versions prior to that as well.

But:

>  drivers/staging/greybus/audio_topology.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> index 76146f91cddc..35775067897c 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1009,6 +1009,19 @@ static const struct snd_soc_dapm_widget gbaudio_widgets[] = {
>  					SND_SOC_DAPM_POST_PMD),
>  };
>  
> +/* Limit %s length to avoid -Wformat-truncation with snprintf() */
> +#define GB_NAME_TMP_LEN 32
> +
> +static void gbaudio_prefix_dev_id(char *name, size_t name_len,
> +				  unsigned int dev_id)
> +{
> +	char temp_name[GB_NAME_TMP_LEN], *cp = temp_name;
> +
> +	strscpy(temp_name, name, sizeof(temp_name));
> +	OPTIMIZER_HIDE_VAR(cp);

What?  Why?  That feels wrong.  Let's not add hacks for broken
compilers.

> +	snprintf(name, name_len, "GB %u %s", dev_id, cp);
> +}
> +
>  static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  				      struct snd_soc_dapm_widget *dw,
>  				      struct gb_audio_widget *w, int *w_size)
> @@ -1018,7 +1031,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  	struct gb_audio_control *curr;
>  	struct gbaudio_control *control, *_control;
>  	size_t size;
> -	char temp_name[NAME_SIZE];
>  
>  	ret = gbaudio_validate_kcontrol_count(w);
>  	if (ret) {
> @@ -1086,8 +1098,7 @@ 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);
> +	gbaudio_prefix_dev_id(w->name, sizeof(w->name), module->dev_id);

This feels like a broken tool, let's not do foolish things just to make
compilers quiet.  W=1 is not a good reason to just make things "silent"
by moving code around like you did here.

sorry,

greg k-h