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>
---
 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

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@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.


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
>

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
> >

On Thu, 06 Nov 2025 12:31:21 +0100,
Lizhi Xu wrote:
> 
> 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;

Hm, so the problem is rather the calculation of the buffer size.
The size sounds extremely small.  Which parameters (rates, formats,
etc) are used for achieving this?

The calculation of u->buffer_size is a bit complex, as maxsize is
adjusted in many different ways.  Is it limited due to wMaxPacketSize
setup?


thanks,

Takashi

On Thu, 06 Nov 2025 12:49:51 +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;
> 
> Hm, so the problem is rather the calculation of the buffer size.
> The size sounds extremely small.  Which parameters (rates, formats,
> etc) are used for achieving this?
rates: 22050
format: 2
channels: 1
/////////////////////////////
stride: 2
packets: 6
data interval: 0
frame_bits: 16
> 
> The calculation of u->buffer_size is a bit complex, as maxsize is
> adjusted in many different ways.  Is it limited due to wMaxPacketSize
> setup?
Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
value is 9.

BR,
Lizhi

On Thu, 06 Nov 2025 15:35:06 +0100,
Lizhi Xu wrote:
> 
> On Thu, 06 Nov 2025 12:49:51 +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;
> > 
> > Hm, so the problem is rather the calculation of the buffer size.
> > The size sounds extremely small.  Which parameters (rates, formats,
> > etc) are used for achieving this?
> rates: 22050
> format: 2
> channels: 1
> /////////////////////////////
> stride: 2
> packets: 6
> data interval: 0
> frame_bits: 16
> > 
> > The calculation of u->buffer_size is a bit complex, as maxsize is
> > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > setup?
> Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> value is 9.

OK, then a fix like below would work?


thanks,

Takashi

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
 	ep->sample_rem = ep->cur_rate % ep->pps;
 	ep->packsize[0] = ep->cur_rate / ep->pps;
 	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
+	if (ep->packsize[1] > ep->maxpacksize) {
+		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
+			      ep->maxpacksize, ep->cur_rate, ep->pps);
+		return -EINVAL;
+	}
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;

On Thu, 06 Nov 2025 17:41:07 +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;
> > >
> > > Hm, so the problem is rather the calculation of the buffer size.
> > > The size sounds extremely small.  Which parameters (rates, formats,
> > > etc) are used for achieving this?
> > rates: 22050
> > format: 2
> > channels: 1
> > /////////////////////////////
> > stride: 2
> > packets: 6
> > data interval: 0
> > frame_bits: 16
> > >
> > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > setup?
> > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > value is 9.
> 
> OK, then a fix like below would work?
> 
> 
> thanks,
> 
> Takashi
> 
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
>  	ep->sample_rem = ep->cur_rate % ep->pps;
>  	ep->packsize[0] = ep->cur_rate / ep->pps;
>  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> +	if (ep->packsize[1] > ep->maxpacksize) {
> +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> +		return -EINVAL;
> +	}
> 
>  	/* calculate the frequency in 16.16 format */
>  	ep->freqm = ep->freqn;
Of course, this fix was added after packsize[0] was assigned a value,
and Hillf Danton has already tested it.

However, to be more precise, although both packsize[1] and packsize[0]
exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
is more rigorous.

BR,
Lizhi

On Fri, 7 Nov 2025 08:54:20 +0800, 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@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;
> > > >
> > > > Hm, so the problem is rather the calculation of the buffer size.
> > > > The size sounds extremely small.  Which parameters (rates, formats,
> > > > etc) are used for achieving this?
> > > rates: 22050
> > > format: 2
> > > channels: 1
> > > /////////////////////////////
> > > stride: 2
> > > packets: 6
> > > data interval: 0
> > > frame_bits: 16
> > > >
> > > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > > setup?
> > > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > > value is 9.
> > 
> > OK, then a fix like below would work?
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
> >  	ep->sample_rem = ep->cur_rate % ep->pps;
> >  	ep->packsize[0] = ep->cur_rate / ep->pps;
> >  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> > +	if (ep->packsize[1] > ep->maxpacksize) {
> > +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> > +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> > +		return -EINVAL;
> > +	}
> > 
> >  	/* calculate the frequency in 16.16 format */
> >  	ep->freqm = ep->freqn;
> Of course, this fix was added after packsize[0] was assigned a value,
> and Hillf Danton has already tested it.
> 
> However, to be more precise, although both packsize[1] and packsize[0]
> exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
> is more rigorous.
Of course, since packsize[1] is always greater than packsize[0] when pps
is greater than 1, judging only packsize[1] is sufficient to avoid judging
packsize[0] and packsize[1].

BR,
Lizhi

On Fri, 07 Nov 2025 02:13:27 +0100,
Lizhi Xu wrote:
> 
> On Fri, 7 Nov 2025 08:54:20 +0800, 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@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;
> > > > >
> > > > > Hm, so the problem is rather the calculation of the buffer size.
> > > > > The size sounds extremely small.  Which parameters (rates, formats,
> > > > > etc) are used for achieving this?
> > > > rates: 22050
> > > > format: 2
> > > > channels: 1
> > > > /////////////////////////////
> > > > stride: 2
> > > > packets: 6
> > > > data interval: 0
> > > > frame_bits: 16
> > > > >
> > > > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > > > setup?
> > > > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > > > value is 9.
> > > 
> > > OK, then a fix like below would work?
> > > 
> > > 
> > > thanks,
> > > 
> > > Takashi
> > > 
> > > --- a/sound/usb/endpoint.c
> > > +++ b/sound/usb/endpoint.c
> > > @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
> > >  	ep->sample_rem = ep->cur_rate % ep->pps;
> > >  	ep->packsize[0] = ep->cur_rate / ep->pps;
> > >  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> > > +	if (ep->packsize[1] > ep->maxpacksize) {
> > > +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> > > +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> > > +		return -EINVAL;
> > > +	}
> > > 
> > >  	/* calculate the frequency in 16.16 format */
> > >  	ep->freqm = ep->freqn;
> > Of course, this fix was added after packsize[0] was assigned a value,
> > and Hillf Danton has already tested it.
> > 
> > However, to be more precise, although both packsize[1] and packsize[0]
> > exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
> > is more rigorous.
> Of course, since packsize[1] is always greater than packsize[0] when pps
> is greater than 1, judging only packsize[1] is sufficient to avoid judging
> packsize[0] and packsize[1].

Right, that's the reason it checks only packsize[1].

I think it's fine to add another check like your patch in addition for
more safety, too.  But then it should better in copy_to_urb() for
both size and offset.  And, it should be with WARN_ON() or such, as
the OOB must not happen after packsize[] sanity check.


thanks,

Takashi

On Thu, 06 Nov 2025 17:41:07 +0100 Takashi Iwai wrote:
> OK, then a fix like below would work?

Test Takashi's fix.

#syz test

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
 	ep->sample_rem = ep->cur_rate % ep->pps;
 	ep->packsize[0] = ep->cur_rate / ep->pps;
 	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
+	if (ep->packsize[1] > ep->maxpacksize) {
+		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
+			      ep->maxpacksize, ep->cur_rate, ep->pps);
+		return -EINVAL;
+	}
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;
--

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com

Tested on:

commit:         a1388fcb Merge tag 'libcrypto-for-linus' of git://git...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1581d812580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=10ae2a58580000

Note: testing is done by a robot and is best-effort only.