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 <[email protected]> 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/[email protected]?part=5
