Re: [PATCH] ALSA: asihpi: use scnprintf() for control name formatting

[Takashi Iwai <tiwai@suse.de>]

On Sat, 28 Mar 2026 00:48:48 +0100,
Pengpeng Hou wrote:
> 
> asihpi_ctl_init() builds mixer control names in the fixed 44-byte
> hpi_ctl->name buffer with sprintf().
> 
> This is not only a defensive cleanup. The current in-tree name tables and
> format strings can already exceed 44 bytes. For example,
> 
>   "Bitstream 0 Internal 0 Monitor Playback Volume"
> 
> is 46 characters before the trailing NUL, so the current sprintf() call
> writes past the end of hpi_ctl->name.
> 
> Switch the formatting to scnprintf() so the driver truncates control
> names instead of overflowing the fixed ALSA control name storage.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

Thanks, now it's clearer.  But if the string overflow can happen
really, it's rather a bigger problem; then we can't blindly truncate
the string as it's used as the control API element key.

I guess this didn't happen in reality because the choice of source and
destination depends on the firmware and the selection like the above
didn't exist so far in the provided firmware data.  But, we still
should address the problem, of course.

Anyways, if the truncation happens, we'd rather need to notify users
that it's basically an error.  That is, we should use snprintf() in
this case and have a return value check.  If it's truncated, the
driver spews the error, showing the truncated string.

Could you revise the patch in that way and resubmit v3?


thanks,

Takashi


> ---
>  sound/pci/asihpi/asihpi.c | 31 +++++++++++++++++--------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
> index 3a64d0562803..de2859eb3e9f 100644
> --- a/sound/pci/asihpi/asihpi.c
> +++ b/sound/pci/asihpi/asihpi.c
> @@ -1384,22 +1384,25 @@ static void asihpi_ctl_init(struct snd_kcontrol_new *snd_control,
>  		dir = "Playback "; /* PCM Playback source, or  output node */
>  
>  	if (hpi_ctl->src_node_type && hpi_ctl->dst_node_type)
> -		sprintf(hpi_ctl->name, "%s %d %s %d %s%s",
> -			asihpi_src_names[hpi_ctl->src_node_type],
> -			hpi_ctl->src_node_index,
> -			asihpi_dst_names[hpi_ctl->dst_node_type],
> -			hpi_ctl->dst_node_index,
> -			dir, name);
> +		scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name),
> +			  "%s %d %s %d %s%s",
> +			  asihpi_src_names[hpi_ctl->src_node_type],
> +			  hpi_ctl->src_node_index,
> +			  asihpi_dst_names[hpi_ctl->dst_node_type],
> +			  hpi_ctl->dst_node_index,
> +			  dir, name);
>  	else if (hpi_ctl->dst_node_type) {
> -		sprintf(hpi_ctl->name, "%s %d %s%s",
> -		asihpi_dst_names[hpi_ctl->dst_node_type],
> -		hpi_ctl->dst_node_index,
> -		dir, name);
> +		scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name),
> +			  "%s %d %s%s",
> +			  asihpi_dst_names[hpi_ctl->dst_node_type],
> +			  hpi_ctl->dst_node_index,
> +			  dir, name);
>  	} else {
> -		sprintf(hpi_ctl->name, "%s %d %s%s",
> -		asihpi_src_names[hpi_ctl->src_node_type],
> -		hpi_ctl->src_node_index,
> -		dir, name);
> +		scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name),
> +			  "%s %d %s%s",
> +			  asihpi_src_names[hpi_ctl->src_node_type],
> +			  hpi_ctl->src_node_index,
> +			  dir, name);
>  	}
>  }
>  
> -- 
> 2.50.1 (Apple Git-155)
>