[PATCH v4 0/2] ALSA: usx2y: Bound the PCM buffer by the URB size

[PATCH v4 0/2] ALSA: usx2y: Bound the PCM buffer by the URB size

[Sinan Turunc]

v4 of the usx2y buffer fix. Patch 1 changes only its comments and the
nrpacks note in the changelog. Patch 2 now also sizes the playback ISO
descriptors from whole frames, after the review of v3 found that they
were filled from the raw capture actual_length while
transfer_buffer_length used whole frames, and gives the maxpacksize
check its own message.

Patch 1 replaces the two-packet BUFFER_BYTES bound of v2 with a
two-URB BUFFER_SIZE bound in frames that does not read the endpoint,
and keeps the capture drop guard. Patch 2 adds guards in the playback
prepare path for a URB larger than the buffer and a packet larger than
the playback endpoint's maxpacksize, and sizes each descriptor as
counts * stride, all pre-existing and unchecked.

Tested on 7.3-rc7 with a USB/IP device presenting the US-122 interface
layout. The playback case of patch 2 reproduces on the unpatched driver
with ordinary 48-frame packets and a 100-frame buffer, which the stock
constraints allow. With both patches, buffers below the bound are
refused at hw_params and 400-frame buffers stream in both directions
with no KASAN report, for S16_LE and, on capture, S24_3LE. With
195-byte capture packets the descriptor sizing of patch 2 shows through
USB/IP's completion check, which refused the v3 URBs (descriptors of
780 bytes for 768 bytes of data) and accepts the v4 ones. The results
are in the patch messages.

Sinan Turunc (2):
  ALSA: usx2y: Require a buffer of two URBs
  ALSA: usx2y: Guard the playback copy

 sound/usb/usx2y/usbusx2yaudio.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

[PATCH v4 1/2] ALSA: usx2y: Require a buffer of two URBs

[Sinan Turunc]

usx2y_urb_capt_retire() wraps the ring buffer once per packet and
usx2y_urb_play_prepare() once per URB of nr_of_packs() packets of up to
50 frames. Both assume that the data fits in the buffer, and the hw
constraints only require about two 1 ms periods, so a packet larger
than the buffer on capture, or a URB larger than the buffer on playback,
violates that assumption.

Constrain SNDRV_PCM_HW_PARAM_BUFFER_SIZE in snd_usx2y_pcm_open() to at
least two URBs, 2 * nr_of_packs() * 50 frames, for both directions. The
bound is in frames and derived from the driver's own packet limit, so it
does not read the endpoint, whose altsetting is only selected in
prepare. Two URBs rather than one, because the position advances by a
URB on playback and only moves when a URB is less than the whole buffer.
Keep a guard in usx2y_urb_capt_retire() that drops a packet larger than
the buffer, for the case the constraint makes unreachable.

With nrpacks above about 163 (S16_LE) or 109 (S24_3LE) the bound exceeds
the 64 KiB buffer the substream preallocates (twice that with the
preallocation raised to its 128 KiB maximum), so hw_params fails for
those module settings. That is intended: the driver cannot be safe with
a URB larger than half the largest buffer.

Reproduced on 7.3-rc7 with a USB/IP device presenting the US-122
interface layout and 1024-byte packets into a 512-byte capture 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

Tested on 7.3-rc7 with the same device, S16_LE at 44100 Hz, nrpacks 4,
so the bound is 400 frames. A 128-frame capture buffer and a 200-frame
playback buffer are refused at hw_params with EINVAL. A 400-frame buffer
streams for 30 s in each direction with the position advancing inside
the buffer and no KASAN report; the capture guard did not fire. The
same 400-frame capture buffer with S24_3LE, which selects altsetting 2
in prepare, streams for 30 s with the same result.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Sinan Turunc <enscrpt@pm.me>
---
v4: the comment in snd_usx2y_pcm_open() says which direction needs which
bound; the nrpacks limit in the changelog is computed against the
preallocated buffer, not the hw struct maximum (review of v3).
v3: bound the buffer in frames by two URBs instead of two packets, so
the playback copy is covered and the bound no longer reads the endpoint
before the altsetting is selected (both from the review of v2). The
open() comment that described playback as wrapping per packet is
corrected. Both directions tested on 7.3-rc7, see above.
v2: constrain the buffer at open instead of clamping in the copy.

 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..1e1d0cfbd16 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -51,6 +51,18 @@ 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 URBs, so this cannot happen for a device that keeps its
+		 * packets at 50 frames. Drop the packet rather than exceed 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) {
@@ -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);
+	/*
+	 * Capture wraps the ring buffer once per packet and playback once
+	 * per URB of nr_of_packs() packets of at most 50 frames (see
+	 * usx2y_urb_play_prepare()). Playback needs the buffer to hold more
+	 * than one URB and capture more than one packet; use the URB bound
+	 * for both, and two URBs rather than one, because the position only
+	 * advances when a URB is less than the whole buffer. The bound is in
+	 * frames and does not depend on the altsetting, which is selected
+	 * in prepare.
+	 */
+	snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_BUFFER_SIZE,
+				     2 * nr_of_packs() * 50, UINT_MAX);
 	return 0;
 }
 
-- 
2.56.0

[PATCH v4 2/2] ALSA: usx2y: Guard the playback copy

[Sinan Turunc]

usx2y_urb_play_prepare() sums the packet lengths of a whole URB, copies
that many frames with a single wrap, and sizes each playback packet from
the capture packet's actual_length. Three things are unchecked: that the
URB fits in the ring buffer, that a packet fits in the playback
endpoint's maxpacksize, from which tmpbuf is allocated, and that
actual_length is a whole number of frames. The ISO descriptors take the
raw actual_length while transfer_buffer_length and the copies take
count * stride, so a packet that is not a multiple of the stride makes
the descriptors add up to more than the buffer, by up to stride - 1
bytes per packet.

Return -EPIPE, as the existing packet-size check does, when the URB is
larger than the buffer or a packet is larger than maxpacksize, and size
each descriptor from whole frames, so the descriptors add up to
transfer_buffer_length and each stays within maxpacksize. The buffer
case is prevented by the previous patch and this is the guard for it;
the other two depend only on the device.

The first case is reachable on the unpatched driver with ordinary
packets: on 7.3-rc7, a USB/IP device presenting the US-122 interface
layout and sending 48-frame packets, with a 100-frame playback buffer
(two 50-frame periods, inside the stock constraints), 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

With both patches, the same device and a 200-frame playback buffer is
refused at hw_params with EINVAL, and a 400-frame buffer plays for 30 s
with the position advancing inside the buffer, no KASAN report and
neither guard message. The maxpacksize check is not exercised: the
emulated device reports the same wMaxPacketSize on both endpoints.

The descriptor case is visible with capture packets of 195 bytes at
stride 4 (48 frames and 3 bytes): without this change the four
descriptors add up to 780 bytes for 768 bytes of data, and USB/IP's
completion check refuses the URB ("total length of iso packets 780 not
equal to actual length of buffer 768", -EPROTO), after which the driver
stops the streams. With this patch every descriptor is 192 bytes, the
descriptors add up to the data in every URB for 30 s, the position
advances inside the buffer and KASAN reports nothing. The 48-frame
playback run above is unchanged.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Sinan Turunc <enscrpt@pm.me>
---
v4: size the ISO descriptors from whole frames, so they add up to
transfer_buffer_length (review of v3); the maxpacksize check has its own
ratelimited message. Regenerated on top of 1/2.
v3: new patch, split out of the v3 draft; the review of v2 found both
unchecked assumptions. The buffer case is confirmed on the unpatched
driver and the guards tested on 7.3-rc7, see above.

 sound/usb/usx2y/usbusx2yaudio.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c
index 1e1d0cfbd16..798c9bb5386 100644
--- a/sound/usb/usx2y/usbusx2yaudio.c
+++ b/sound/usb/usx2y/usbusx2yaudio.c
@@ -118,14 +118,31 @@ static int usx2y_urb_play_prepare(struct snd_usx2y_substream *subs,
 				__func__, counts);
 			return -EPIPE;
 		}
+		if (counts * usx2y->stride > subs->maxpacksize) {
+			dev_err_ratelimited(&usx2y->dev->dev,
+					    "%s: packet of %i bytes exceeds maxpacksize %u\n",
+					    __func__, counts * usx2y->stride,
+					    subs->maxpacksize);
+			return -EPIPE;
+		}
 		/* set up descriptor */
 		urb->iso_frame_desc[pack].offset = pack ?
 			urb->iso_frame_desc[pack - 1].offset +
 			urb->iso_frame_desc[pack - 1].length :
 			0;
-		urb->iso_frame_desc[pack].length = cap_urb->iso_frame_desc[pack].actual_length;
+		urb->iso_frame_desc[pack].length = counts * usx2y->stride;
 	}
 	if (atomic_read(&subs->state) >= STATE_PRERUNNING) {
+		/*
+		 * The copy below wraps once, which needs count <= buffer_size.
+		 * snd_usx2y_pcm_open() guarantees it; guard anyway.
+		 */
+		if (unlikely(count > runtime->buffer_size)) {
+			dev_err_ratelimited(&usx2y->dev->dev,
+					    "%s: URB of %i frames exceeds %lu-frame buffer, stopping\n",
+					    __func__, count, runtime->buffer_size);
+			return -EPIPE;
+		}
 		if (subs->hwptr + count > runtime->buffer_size) {
 			/* err, the transferred area goes over buffer boundary.
 			 * copy the data to the temp buffer.
-- 
2.56.0

Re: [PATCH v4 0/2] ALSA: usx2y: Bound the PCM buffer by the URB size

[Takashi Iwai]

On Wed, 04 Nov 2026 13:21:52 +0100,
Sinan Turunc wrote:
> 
> v4 of the usx2y buffer fix. Patch 1 changes only its comments and the
> nrpacks note in the changelog. Patch 2 now also sizes the playback ISO
> descriptors from whole frames, after the review of v3 found that they
> were filled from the raw capture actual_length while
> transfer_buffer_length used whole frames, and gives the maxpacksize
> check its own message.
> 
> Patch 1 replaces the two-packet BUFFER_BYTES bound of v2 with a
> two-URB BUFFER_SIZE bound in frames that does not read the endpoint,
> and keeps the capture drop guard. Patch 2 adds guards in the playback
> prepare path for a URB larger than the buffer and a packet larger than
> the playback endpoint's maxpacksize, and sizes each descriptor as
> counts * stride, all pre-existing and unchecked.
> 
> Tested on 7.3-rc7 with a USB/IP device presenting the US-122 interface
> layout. The playback case of patch 2 reproduces on the unpatched driver
> with ordinary 48-frame packets and a 100-frame buffer, which the stock
> constraints allow. With both patches, buffers below the bound are
> refused at hw_params and 400-frame buffers stream in both directions
> with no KASAN report, for S16_LE and, on capture, S24_3LE. With
> 195-byte capture packets the descriptor sizing of patch 2 shows through
> USB/IP's completion check, which refused the v3 URBs (descriptors of
> 780 bytes for 768 bytes of data) and accepts the v4 ones. The results
> are in the patch messages.
> 
> Sinan Turunc (2):
>   ALSA: usx2y: Require a buffer of two URBs
>   ALSA: usx2y: Guard the playback copy

Applied both patches now.  Thanks.


Takashi