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

[Lizhi Xu <lizhi.xu@windriver.com>]

On Thu, 06 Nov 2025 11:02:08 +0100, Takashi Iwai 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@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.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.
During debugging, I discovered that the value of ep->packsize[0] is 22,
which causes the counts calculated by 
counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
to be 22, resulting in a frames value of 22 * 6 = 132; 
Meanwhile, the stride value is 2, which ultimately results in 
bytes = frames * stride = 132 * 2 = 264;
@@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
	u->buffer_size = maxsize * u->packets;
	...
	u->urb->transfer_buffer =
                usb_alloc_coherent(chip->dev, u->buffer_size,
                                   GFP_KERNEL, &u->urb->transfer_dma);

Here, when calculating u->buffer_size = maxsize * u->packets;
maxsize = 9, packets = 6, which results in only 54 bytes allocated to
transfer_buffer;

BR,
Lizhi
> > ---
> >  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
> >