public inbox for linux-sound@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: caiaq: fix stack out-of-bounds read in init_card
@ 2026-03-29 13:38 Berk Cem Goksel
  2026-03-30  7:33 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Berk Cem Goksel @ 2026-03-29 13:38 UTC (permalink / raw)
  To: zonque, tiwai; +Cc: linux-sound, stable, andreyknvl, Berk Cem Goksel

The loop creates a whitespace-stripped copy of the card shortname
where `len < sizeof(card->id)` is used for the bounds check. Since
sizeof(card->id) is 16 and the local id buffer is also 16 bytes,
writing 16 non-space characters fills the entire buffer,
overwriting the terminating nullbyte.

When this non-null-terminated string is later passed to
snd_card_set_id() -> copy_valid_id_string(), the function scans
forward with `while (*nid && ...)` and reads past the end of the
stack buffer, reading the contents of the stack.

A USB device with a product name containing many non-ASCII, non-space
characters (e.g. multibyte UTF-8) will reliably trigger this as follows:

  BUG: KASAN: stack-out-of-bounds in copy_valid_id_string
       sound/core/init.c:696 [inline]
  BUG: KASAN: stack-out-of-bounds in snd_card_set_id_no_lock+0x698/0x74c
       sound/core/init.c:718

The off-by-one has been present since commit bafeee5b1f8d ("ALSA:
snd_usb_caiaq: give better shortname") from June 2009 (v2.6.31-rc1),
which first introduced this whitespace-stripping loop. The original
code never accounted for the null terminator when bounding the copy.

Fix this by changing the loop bound to `sizeof(card->id) - 1`,
ensuring at least one byte remains as the null terminator.

Fixes: bafeee5b1f8d ("ALSA: snd_usb_caiaq: give better shortname")
Cc: stable@vger.kernel.org
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Reported-by: Berk Cem Goksel <berkcgoksel@gmail.com>
Signed-off-by: Berk Cem Goksel <berkcgoksel@gmail.com>
---
Tested on 6.19.0 and 7.0.0-rc5 with KASAN enabled (arm64, dummy_hcd):

[  302.559633] BUG: KASAN: stack-out-of-bounds in snd_card_set_id_no_lock+0x698/0x74c
[  302.559663] Read of size 1 at addr ffff8000a3ff6b10 by task kworker/1:4/2208
[  302.559701] Hardware name: linux,dummy-virt (DT)
[  302.559709] Workqueue: usb_hub_wq hub_event
[  302.559727] Call trace:
[  302.559751]  dump_stack_lvl+0x138/0x1c8
[  302.559793]  kasan_report+0xc0/0x100
[  302.559833]  __asan_report_load1_noabort+0x20/0x2c
[  302.559853]  snd_card_set_id_no_lock+0x698/0x74c
[  302.559873]  snd_card_set_id+0xa0/0xe4
[  302.559891]  snd_probe+0xc34/0xf04
[  302.559906]  usb_probe_interface+0x2c4/0x998

 sound/usb/caiaq/device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/usb/caiaq/device.c b/sound/usb/caiaq/device.c
index dfd820483..3a71bab8a 100644
--- a/sound/usb/caiaq/device.c
+++ b/sound/usb/caiaq/device.c
@@ -488,7 +488,7 @@ static int init_card(struct snd_usb_caiaqdev *cdev)
 		memset(id, 0, sizeof(id));

 		for (c = card->shortname, len = 0;
-			*c && len < sizeof(card->id); c++)
+			*c && len < sizeof(card->id) - 1; c++)
 			if (*c != ' ')
 				id[len++] = *c;

--
2.34.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ALSA: caiaq: fix stack out-of-bounds read in init_card
  2026-03-29 13:38 [PATCH] ALSA: caiaq: fix stack out-of-bounds read in init_card Berk Cem Goksel
@ 2026-03-30  7:33 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-03-30  7:33 UTC (permalink / raw)
  To: Berk Cem Goksel; +Cc: zonque, tiwai, linux-sound, stable, andreyknvl

On Sun, 29 Mar 2026 15:38:25 +0200,
Berk Cem Goksel wrote:
> 
> The loop creates a whitespace-stripped copy of the card shortname
> where `len < sizeof(card->id)` is used for the bounds check. Since
> sizeof(card->id) is 16 and the local id buffer is also 16 bytes,
> writing 16 non-space characters fills the entire buffer,
> overwriting the terminating nullbyte.
> 
> When this non-null-terminated string is later passed to
> snd_card_set_id() -> copy_valid_id_string(), the function scans
> forward with `while (*nid && ...)` and reads past the end of the
> stack buffer, reading the contents of the stack.
> 
> A USB device with a product name containing many non-ASCII, non-space
> characters (e.g. multibyte UTF-8) will reliably trigger this as follows:
> 
>   BUG: KASAN: stack-out-of-bounds in copy_valid_id_string
>        sound/core/init.c:696 [inline]
>   BUG: KASAN: stack-out-of-bounds in snd_card_set_id_no_lock+0x698/0x74c
>        sound/core/init.c:718
> 
> The off-by-one has been present since commit bafeee5b1f8d ("ALSA:
> snd_usb_caiaq: give better shortname") from June 2009 (v2.6.31-rc1),
> which first introduced this whitespace-stripping loop. The original
> code never accounted for the null terminator when bounding the copy.
> 
> Fix this by changing the loop bound to `sizeof(card->id) - 1`,
> ensuring at least one byte remains as the null terminator.
> 
> Fixes: bafeee5b1f8d ("ALSA: snd_usb_caiaq: give better shortname")
> Cc: stable@vger.kernel.org
> Cc: Andrey Konovalov <andreyknvl@gmail.com>
> Reported-by: Berk Cem Goksel <berkcgoksel@gmail.com>
> Signed-off-by: Berk Cem Goksel <berkcgoksel@gmail.com>

Applied now.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-03-30  7:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-29 13:38 [PATCH] ALSA: caiaq: fix stack out-of-bounds read in init_card Berk Cem Goksel
2026-03-30  7:33 ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox