mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ALSA: usb-audio: Drop capture packets with bogus actual_length
       [not found] <CGME20261110060032epcas1p1a4de057d86d384387d0b8004d63ef331@epcas1p1.samsung.com>
@ 2026-11-10  6:00 ` Gichoon Han
  2026-11-10 10:46   ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Gichoon Han @ 2026-11-10  6:00 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela
  Cc: Sinan Turunc, Edward Adam Davis, linux-sound, linux-kernel,
	Gichoon Han, stable

retire_capture_urb() copies urb->iso_frame_desc[i].actual_length bytes
out of urb->transfer_buffer at the packet's offset, but never checks
that value against the requested packet length.  A host controller
reporting more data than requested makes memcpy() read past the end
of the URB transfer buffer.

Commit 06be918062ad ("ALSA: usb-audio: Bound packet copies by the PCM
buffer size") clamps the count to the PCM ring buffer size, which
protects the destination.  The source slot is only ep->curpacksize
bytes per packet, though, so the over-read remains.

This was hit on an ARM32 system (vendor 5.4 kernel; the code path is
unchanged in mainline) with a full-speed UAC1 microphone behind a
high-speed hub, i.e. the EHCI siTD path.  sitd_complete() computes
actual_length as desc->length - SITD_LENGTH(t); the controller
reported a remaining length larger than the programmed one, the value
wrapped to ~4G, and the capture copy faulted on the first unmapped
page after the URB buffer:

  Unable to handle kernel paging request at virtual address e8756000
  Internal error: Oops: 7 [#1] PREEMPT SMP ARM
  r3 : 00000000  r2 : 0001e700  r1 : e8756000  r0 : e878fc80
  Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8)

A packet can never legitimately carry more than its requested length,
so warn and skip it instead of copying.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Gichoon Han <gichoon.han@samsung.com>
---
Compile-tested only on linux-next.  The crash was observed in the
field on a vendor 5.4-based ARM32 kernel with the same code path;
I don't have a setup to reproduce the bogus siTD length on mainline.

 sound/usb/pcm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index c2f7b08607ac..57a26492dd61 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1337,6 +1337,12 @@ static void retire_capture_urb(struct snd_usb_substream *subs,
 					    "frame %d active: %d\n", i,
 					    urb->iso_frame_desc[i].status);
 		bytes = urb->iso_frame_desc[i].actual_length;
+		if (unlikely(bytes > urb->iso_frame_desc[i].length)) {
+			dev_warn_ratelimited(&subs->dev->dev,
+					     "packet %d: actual_length %u exceeds length %u, dropping\n",
+					     i, bytes, urb->iso_frame_desc[i].length);
+			continue;
+		}
 		if (subs->stream_offset_adj > 0) {
 			unsigned int adj = min(subs->stream_offset_adj, bytes);
 			cp += adj;

base-commit: 630059a5a632377b9b7b487baccde92b7ae1dd47
-- 
2.25.1


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

* Re: [PATCH] ALSA: usb-audio: Drop capture packets with bogus actual_length
  2026-11-10  6:00 ` [PATCH] ALSA: usb-audio: Drop capture packets with bogus actual_length Gichoon Han
@ 2026-11-10 10:46   ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-11-10 10:46 UTC (permalink / raw)
  To: Gichoon Han
  Cc: Takashi Iwai, Jaroslav Kysela, Sinan Turunc, Edward Adam Davis,
	linux-sound, linux-kernel, stable

On Tue, 10 Nov 2026 07:00:21 +0100,
Gichoon Han wrote:
> 
> retire_capture_urb() copies urb->iso_frame_desc[i].actual_length bytes
> out of urb->transfer_buffer at the packet's offset, but never checks
> that value against the requested packet length.  A host controller
> reporting more data than requested makes memcpy() read past the end
> of the URB transfer buffer.
> 
> Commit 06be918062ad ("ALSA: usb-audio: Bound packet copies by the PCM
> buffer size") clamps the count to the PCM ring buffer size, which
> protects the destination.  The source slot is only ep->curpacksize
> bytes per packet, though, so the over-read remains.
> 
> This was hit on an ARM32 system (vendor 5.4 kernel; the code path is
> unchanged in mainline) with a full-speed UAC1 microphone behind a
> high-speed hub, i.e. the EHCI siTD path.  sitd_complete() computes
> actual_length as desc->length - SITD_LENGTH(t); the controller
> reported a remaining length larger than the programmed one, the value
> wrapped to ~4G, and the capture copy faulted on the first unmapped
> page after the URB buffer:
> 
>   Unable to handle kernel paging request at virtual address e8756000
>   Internal error: Oops: 7 [#1] PREEMPT SMP ARM
>   r3 : 00000000  r2 : 0001e700  r1 : e8756000  r0 : e878fc80
>   Code: ba000002 f5d1f03c f5d1f05c f5d1f07c (e8b151f8)
> 
> A packet can never legitimately carry more than its requested length,
> so warn and skip it instead of copying.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gichoon Han <gichoon.han@samsung.com>
> ---
> Compile-tested only on linux-next.  The crash was observed in the
> field on a vendor 5.4-based ARM32 kernel with the same code path;
> I don't have a setup to reproduce the bogus siTD length on mainline.

I think the bug is rather in the USB contorller code, but it's still
safer to have a sanity check.  So I applied now.


thanks,

Takashi

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

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

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20261110060032epcas1p1a4de057d86d384387d0b8004d63ef331@epcas1p1.samsung.com>
2026-11-10  6:00 ` [PATCH] ALSA: usb-audio: Drop capture packets with bogus actual_length Gichoon Han
2026-11-10 10:46   ` Takashi Iwai

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®