mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets
@ 2026-11-03 13:23 Sinan Turunc
  2026-11-04  7:19 ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Sinan Turunc @ 2026-11-03 13:23 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai; +Cc: linux-sound, linux-kernel, stable

usx2y_urb_capt_retire() splits the copy into the ALSA ring buffer once
at the buffer end and wraps hwptr_done once, which is only correct when
one packet holds at most buffer_size frames. The packet size is the
endpoint's maxpacksize from usb_maxpacket(), while the hw constraints
allow 64-byte periods with periods_min 2, so a packet can be larger than
the whole buffer.

The endpoint is known at probe time, so constrain
SNDRV_PCM_HW_PARAM_BUFFER_BYTES in snd_usx2y_pcm_open() to at least
twice the endpoint's packet size. Two packets, matching the line6 fix.
The bound makes the copy safe, but it does not make every larger buffer
progress: the position is reported once per URB of 4 packets, so a
buffer that divides one URB's data returns to the same position at every
callback. That was already the case before this patch and is not changed
here. Keep a guard in usx2y_urb_capt_retire() that drops an oversized
packet with a ratelimited error, for the case that should no longer be
reachable.

Reproduced on 7.3-rc7 with a USB/IP device presenting the US-122
interface layout and 1024-byte packets into a 512-byte buffer:

  BUG: KASAN: out-of-bounds in i_usx2y_urb_complete+0x1285/0x1ac0
  Read of size 18446744073709551104 at addr ffff88800e448800 by task vhci_rx/83

With this patch a 1024-byte buffer is refused at hw_params; a 2048-byte
buffer is accepted and safe but barely advances (hw_ptr 9216 after 30 s,
for the reason above); a 3072-byte buffer streams for 30 s with no
report and with the position moving inside the buffer.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Sinan Turunc <enscrpt@pm.me>
---
v2: constrain the buffer at open instead of clamping in the copy. v1
clamped the frame count to buffer_size, which left the ring position
unchanged on every oversized packet and stalled the stream, as the
Sashiko review of the line6 patch pointed out. The bound is two
packets, matching the line6 fix; see the message for what it does and
does not change. The clamp is replaced by a drop guard for the case the
constraint makes unreachable.

 sound/usb/usx2y/usbusx2yaudio.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index a3a0bc13632..d1ddc2b425d 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -51,6 +51,17 @@ static int usx2y_urb_capt_retire(struct snd_usx2y_substream *subs)
 			dev_dbg(&usx2y->dev->dev, "%s: 0 == len ERROR!\n", __func__);
 			continue;
 		}
+		/*
+		 * snd_usx2y_pcm_open() constrains the buffer to hold at least
+		 * two packets, so this cannot happen for a conforming device.
+		 * Drop the packet rather than overrun the single wrap below.
+		 */
+		if (unlikely(len > runtime->buffer_size)) {
+			dev_err_ratelimited(&usx2y->dev->dev,
+					    "%s: packet of %i frames exceeds %lu-frame buffer, dropping\n",
+					    __func__, len, runtime->buffer_size);
+			continue;
+		}
 
 		/* copy a data chunk */
 		if ((hwptr_done + len) > runtime->buffer_size) {
@@ -880,6 +891,7 @@ static int snd_usx2y_pcm_open(struct snd_pcm_substream *substream)
 		((struct snd_usx2y_substream **)
 		 snd_pcm_substream_chip(substream))[substream->stream];
 	struct snd_pcm_runtime	*runtime = substream->runtime;
+	unsigned int		pipe;
 
 	if (subs->usx2y->chip_status & USX2Y_STAT_CHIP_MMAP_PCM_URBS)
 		return -EBUSY;
@@ -888,6 +900,18 @@ static int snd_usx2y_pcm_open(struct snd_pcm_substream *substream)
 	runtime->private_data = subs;
 	subs->pcm_substream = substream;
 	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_TIME, 1000, 200000);
+	/*
+	 * The capture and playback copies wrap the ring buffer once per
+	 * packet and advance the position by the packet, which only works
+	 * when the buffer holds more than one packet. The packet size is the
+	 * endpoint's, so bound the buffer by two packets here.
+	 */
+	pipe = substream->stream == SNDRV_PCM_STREAM_PLAYBACK ?
+		usb_sndisocpipe(subs->usx2y->dev, subs->endpoint) :
+		usb_rcvisocpipe(subs->usx2y->dev, subs->endpoint);
+	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_BYTES,
+				     2 * usb_maxpacket(subs->usx2y->dev, pipe),
+				     UINT_MAX);
 	return 0;
 }
 
-- 
2.56.0


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

* Re: [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets
  2026-11-03 13:23 [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets Sinan Turunc
@ 2026-11-04  7:19 ` Takashi Iwai
  2026-11-04  7:34   ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2026-11-04  7:19 UTC (permalink / raw)
  To: Sinan Turunc
  Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel, stable

On Tue, 03 Nov 2026 14:23:03 +0100,
Sinan Turunc wrote:
> 
> usx2y_urb_capt_retire() splits the copy into the ALSA ring buffer once
> at the buffer end and wraps hwptr_done once, which is only correct when
> one packet holds at most buffer_size frames. The packet size is the
> endpoint's maxpacksize from usb_maxpacket(), while the hw constraints
> allow 64-byte periods with periods_min 2, so a packet can be larger than
> the whole buffer.
> 
> The endpoint is known at probe time, so constrain
> SNDRV_PCM_HW_PARAM_BUFFER_BYTES in snd_usx2y_pcm_open() to at least
> twice the endpoint's packet size. Two packets, matching the line6 fix.
> The bound makes the copy safe, but it does not make every larger buffer
> progress: the position is reported once per URB of 4 packets, so a
> buffer that divides one URB's data returns to the same position at every
> callback. That was already the case before this patch and is not changed
> here. Keep a guard in usx2y_urb_capt_retire() that drops an oversized
> packet with a ratelimited error, for the case that should no longer be
> reachable.
> 
> Reproduced on 7.3-rc7 with a USB/IP device presenting the US-122
> interface layout and 1024-byte packets into a 512-byte buffer:
> 
>   BUG: KASAN: out-of-bounds in i_usx2y_urb_complete+0x1285/0x1ac0
>   Read of size 18446744073709551104 at addr ffff88800e448800 by task vhci_rx/83
> 
> With this patch a 1024-byte buffer is refused at hw_params; a 2048-byte
> buffer is accepted and safe but barely advances (hw_ptr 9216 after 30 s,
> for the reason above); a 3072-byte buffer streams for 30 s with no
> report and with the position moving inside the buffer.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sinan Turunc <enscrpt@pm.me>

Applied now.  thanks.


Takashi

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

* Re: [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets
  2026-11-04  7:19 ` Takashi Iwai
@ 2026-11-04  7:34   ` Takashi Iwai
  2026-11-04 10:52     ` Sinan Turunc
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2026-11-04  7:34 UTC (permalink / raw)
  To: Sinan Turunc
  Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel, stable

On Wed, 04 Nov 2026 08:19:50 +0100,
Takashi Iwai wrote:
> 
> On Tue, 03 Nov 2026 14:23:03 +0100,
> Sinan Turunc wrote:
> > 
> > usx2y_urb_capt_retire() splits the copy into the ALSA ring buffer once
> > at the buffer end and wraps hwptr_done once, which is only correct when
> > one packet holds at most buffer_size frames. The packet size is the
> > endpoint's maxpacksize from usb_maxpacket(), while the hw constraints
> > allow 64-byte periods with periods_min 2, so a packet can be larger than
> > the whole buffer.
> > 
> > The endpoint is known at probe time, so constrain
> > SNDRV_PCM_HW_PARAM_BUFFER_BYTES in snd_usx2y_pcm_open() to at least
> > twice the endpoint's packet size. Two packets, matching the line6 fix.
> > The bound makes the copy safe, but it does not make every larger buffer
> > progress: the position is reported once per URB of 4 packets, so a
> > buffer that divides one URB's data returns to the same position at every
> > callback. That was already the case before this patch and is not changed
> > here. Keep a guard in usx2y_urb_capt_retire() that drops an oversized
> > packet with a ratelimited error, for the case that should no longer be
> > reachable.
> > 
> > Reproduced on 7.3-rc7 with a USB/IP device presenting the US-122
> > interface layout and 1024-byte packets into a 512-byte buffer:
> > 
> >   BUG: KASAN: out-of-bounds in i_usx2y_urb_complete+0x1285/0x1ac0
> >   Read of size 18446744073709551104 at addr ffff88800e448800 by task vhci_rx/83
> > 
> > With this patch a 1024-byte buffer is refused at hw_params; a 2048-byte
> > buffer is accepted and safe but barely advances (hw_ptr 9216 after 30 s,
> > for the reason above); a 3072-byte buffer streams for 30 s with no
> > report and with the position moving inside the buffer.
> > 
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sinan Turunc <enscrpt@pm.me>
> 
> Applied now.  thanks.

... and I noticed Sashiko warnings now, so dropped again from the
queue.  Could you verify whether those complaints are valid or not?


thanks,

Takashi

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

* Re: [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets
  2026-11-04  7:34   ` Takashi Iwai
@ 2026-11-04 10:52     ` Sinan Turunc
  0 siblings, 0 replies; 4+ messages in thread
From: Sinan Turunc @ 2026-11-04 10:52 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, linux-sound, linux-kernel, stable, sashiko-reviews

On Wed, 04 Nov 2026 08:34:19 +0100,
Takashi Iwai wrote:
> ... and I noticed Sashiko warnings now, so dropped again from the
> queue. Could you verify whether those complaints are valid or not?

Yes, both are valid.

The critical one: usx2y_urb_play_prepare() copies a whole URB of
nr_of_packs() packets out of the ring buffer with a single wrap, so with
a buffer smaller than one URB the copy reads past the end of the buffer,
and on a later URB the length goes negative and memcpy() gets a huge
size. This is pre-existing. The stock constraints allow a buffer of two
1 ms periods, below one default URB of up to 200 frames. On 7.3-rc7,
with 48-frame packets, inside the 43 to 50 frames the driver accepts,
and a 100-frame playback buffer, KASAN reports:

  BUG: KASAN: out-of-bounds in i_usx2y_urb_complete+0x79b/0x1ac0
  Read of size 18446744073709551280 at addr ffff88800e6b02e0 by task vhci_rx/80

v2 did not cause it, but its two-packet bound and its comment did not
cover it.

The medium one: v2 read usb_maxpacket() in snd_usx2y_pcm_open(), before
usx2y_format_set() selects the altsetting in prepare, so the bound could
be stale or 0. The capture copy stayed safe through its per-packet check
against runtime->buffer_size; only the bound was unreliable.

v3 replaces the byte bound with a frame bound of two URBs,
2 * nr_of_packs() * 50, which does not depend on the altsetting, and
adds the playback guards as a separate patch. It follows as a two-patch
series. Tested on 7.3-rc7 with an emulated US-122: buffers below the
bound are refused at hw_params, and 400-frame buffers stream in both
directions for S16_LE and on capture for S24_3LE, with no KASAN report.

Thanks,
Sinan


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

end of thread, other threads:[~2026-11-04 10:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-03 13:23 [PATCH v2] ALSA: usx2y: Reject buffers smaller than two packets Sinan Turunc
2026-11-04  7:19 ` Takashi Iwai
2026-11-04  7:34   ` Takashi Iwai
2026-11-04 10:52     ` Sinan Turunc

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

all inboxes | Powered by JetHome®