Re: [PATCH] ALSA: hda/generic: Replace strlcat() with strscpy()

[Takashi Iwai <tiwai@suse.de>]

On Tue, 14 Jul 2026 21:36:37 +0200,
Ian Bridges wrote:
> 
> In preparation for removing the strlcat() API[1], replace its two
> uses in the generic parser.
> 
> Both call sites append one suffix to a string that the function has
> already bounded to the buffer size. A strscpy() anchored at the
> current end of the string writes the same bytes, including when the
> suffix is truncated.
> 
> Link: https://github.com/KSPP/linux/issues/370 [1]
> Signed-off-by: Ian Bridges <icb@fastmail.org>
> ---
> The patch was tested as follows.
> 
> - W=1 build of sound/hda/, zero warnings.
> - A userspace differential harness compiled the old and the new
>   functions side by side. The buffers were byte identical over their
>   full length, including the bytes after the terminator.
> - A QEMU runtime compare. A guest with an emulated HDA codec was
>   booted on the base and on the patched kernel, and the captured
>   /proc/asound state was byte identical. That covers the PCM stream
>   names from fill_pcm_stream_name() live.
> 
>  sound/hda/codecs/generic.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/codecs/generic.c b/sound/hda/codecs/generic.c
> index 660a9f2c0ded..5f373cbf4a53 100644
> --- a/sound/hda/codecs/generic.c
> +++ b/sound/hda/codecs/generic.c
> @@ -2712,10 +2712,12 @@ static void get_jack_mode_name(struct hda_codec *codec, hda_nid_t pin,
>  			       char *name, size_t name_len)
>  {
>  	struct hda_gen_spec *spec = codec->spec;
> +	size_t used;
>  	int idx = 0;
>  
>  	snd_hda_get_pin_label(codec, pin, &spec->autocfg, name, name_len, &idx);
> -	strlcat(name, " Jack Mode", name_len);
> +	used = strnlen(name, name_len);
> +	strscpy(name + used, " Jack Mode", name_len - used);
>  
>  	for (; find_kctl_name(codec, name, idx); idx++)
>  		;
> @@ -5682,6 +5684,7 @@ static const struct hda_pcm_stream dyn_adc_pcm_analog_capture = {
>  static void fill_pcm_stream_name(char *str, size_t len, const char *sfx,
>  				 const char *chip_name)
>  {
> +	size_t used;
>  	char *p;
>  
>  	if (*str)
> @@ -5695,7 +5698,8 @@ static void fill_pcm_stream_name(char *str, size_t len, const char *sfx,
>  			break;
>  		}
>  	}
> -	strlcat(str, sfx, len);
> +	used = strnlen(str, len);
> +	strscpy(str + used, sfx, len - used);
>  }
>  
>  /* copy PCM stream info from @default_str, and override non-NULL entries

Hmm...  IMHO, those changes are *too ugly*.

Is the reason to drop strlcat() is only about the performance, no?
If it were some security hardening, I'd find OK, but if not, this kind
of change is doubtful: the code path is no hot path, hence the effect
is nothing, and yet readability becomes significantly worse.


thanks,

Takashi