Re: [PATCH 05/28] ALSA: usb-audio: Refactor usb_audio_make_longname() to use seq_buf

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] alsa: usb-audio: string corruption in usb_audio_make_longname() on seq_buf overflow
--

commit 4261f1e1f38e25d5a907e1a1ad3aeefb3e4075ae
Author: Bill Wendling <morbo@google.com>

ALSA: usb-audio: Refactor usb_audio_make_longname() to use seq_buf

This commit refactors usb_audio_make_longname() to use struct seq_buf 
instead of deprecated string functions like strlcat() for building the
device longname.

Link: https://github.com/KSPP/linux/issues/370 [1]

> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index 9307da95efbef..bdca8085fca66 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c

[ ... ]

> @@ -667,44 +670,61 @@ static void usb_audio_make_longname(struct usb_device *dev,
>  		s = preset->vendor_name;
>  	else if (quirk && quirk->vendor_name)
>  		s = quirk->vendor_name;
> -	*card->longname = 0;
> +
> +	seq_buf_init(&sb, card->longname, sizeof(card->longname));
> +
>  	if (s && *s)
> -		strscpy(card->longname, s);
> +		seq_buf_puts(&sb, s);
>  	else if (dev->manufacturer && *dev->manufacturer)
> -		strscpy(card->longname, dev->manufacturer);
> -
> -	if (*card->longname) {
> -		strim(card->longname);
> -		if (*card->longname)
> -			strlcat(card->longname, " ", sizeof(card->longname));
> +		seq_buf_puts(&sb, dev->manufacturer);
> +
> +	if (seq_buf_used(&sb)) {
> +		char *trimmed;
> +
> +		seq_buf_str(&sb);
> +		trimmed = strim(card->longname);
> +		if (trimmed != card->longname)
> +			memmove(card->longname, trimmed, strlen(trimmed) + 1);
> +		sb.len = strlen(card->longname);
> +		if (sb.len)
> +			seq_buf_putc(&sb, ' ');

[Severity: Medium]
Does this seq_buf_putc() call leave the string without a null terminator if
the buffer subsequently overflows?

strim() and memmove() remove leading and trailing spaces by shifting the
string and inserting a null terminator, but they leave the old shifted
characters in the buffer immediately after the new null terminator.
seq_buf_putc() then overwrites the new null terminator with a space, but
does not append a new one to maintain string safety.

If any subsequent seq_buf_puts() calls (like shortname, path, or speed)
exceed the 80-byte capacity, they silently abort and place the buffer into
an overflow state without writing a new null terminator. 

At the end of usb_audio_make_longname(), seq_buf_str() sees the overflow
state and places a null terminator only at the very end of the buffer, not
at the logical string end.

Can this expose duplicated characters or trailing spaces when the device
manufacturer string and subsequent concatenated parts exceed 80 bytes?

>  	}
>  
> -	strlcat(card->longname, card->shortname, sizeof(card->longname));
> +	seq_buf_puts(&sb, card->shortname);
>  
> -	len = strlcat(card->longname, " at ", sizeof(card->longname));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915081910.4142719-1-morbo@google.com?part=5