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

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®