linux-kernel - Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a50zh42n.wl-tiwai@suse.de>
Date: Thu, 06 Nov 2025 11:02:08 +0100
From: Takashi Iwai <tiwai@...e.de>
To: Lizhi Xu <lizhi.xu@...driver.com>
Cc: <syzbot+bfd77469c8966de076f7@...kaller.appspotmail.com>,
	<linux-kernel@...r.kernel.org>,
	<linux-sound@...r.kernel.org>,
	<linux-usb@...r.kernel.org>,
	<perex@...ex.cz>,
	<syzkaller-bugs@...glegroups.com>,
	<tiwai@...e.com>
Subject: Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds

On Thu, 06 Nov 2025 09:33:00 +0100,
Lizhi Xu wrote:
> 
> The calculation rule for the actual data length written to the URB's
> transfer buffer differs from that used to allocate the URB's transfer
> buffer, and in this problem, the value used during allocation is smaller.
> 
> This ultimately leads to write out-of-bounds errors when writing data to
> the transfer buffer.
> 
> To prevent out-of-bounds writes to the transfer buffer, a check between
> the size of the bytes to be written and the size of the allocated bytes
> should be added before performing the write operation.
> 
> When the written bytes are too large, -EPIPE is returned instead of
> -EAGAIN, because returning -EAGAIN might result in push back to ready
> list again.
> 
> Based on the context of calculating the bytes to be written here, both
> copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> the bytes to be written before execution.
> 
> syzbot reported:
> BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> 
> Call Trace:
>  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
>  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> 
> Reported-by: syzbot+bfd77469c8966de076f7@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> Tested-by: syzbot+bfd77469c8966de076f7@...kaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@...driver.com>

I'm afraid that this doesn't address the root cause at all.
The description above sounds plausible, but not pointing to "why".

The bytes is frames * stride, so the question is why a too large
frames is calculated.  I couldn't have time to check the details, but
there should be rather some weird condition / parameters to trigger
this, and we should check that at first.


thanks,

Takashi


> ---
>  sound/usb/pcm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 54d01dfd820f..a4c0ea685b8a 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1606,6 +1606,9 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
>  				    subs->cur_audiofmt->dsd_bitrev)) {
>  			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
>  		} else {
> +			if (bytes > ctx->buffer_size)
> +				return -EPIPE;
> +
>  			/* usual PCM */
>  			if (!subs->tx_length_quirk)
>  				copy_to_urb(subs, urb, 0, stride, bytes);
> -- 
> 2.43.0
>