[PATCH] media: au0828: reject too-short isochronous packets in au0828_isoc_copy

[PATCH] media: au0828: reject too-short isochronous packets in au0828_isoc_copy

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

au0828_isoc_copy() strips a 4-byte (or 8-byte, for a field-start packet)
in-band header from each isochronous packet and computes the remaining
payload length:

	len = urb->iso_frame_desc[i].actual_length - 4;
	p += 4;
	if (fbyte & 0x80) {
		len -= 4;
		p += 4;
		...
	}

actual_length is only validated for <= 0 and for exceeding max_pkt_size,
and len is a signed int, so a malicious or malfunctioning USB device that
completes an isochronous IN packet with actual_length 1..3 (or 1..7 for a
field-start packet) makes len negative.

The negative len is then handed to au0828_copy_vbi(), whose length
parameter is unsigned long, so it is sign-extended to a value close to
ULONG_MAX and drives the copy loop:

	for (i = 0; i < len; i += 2)
		startwrite[j++] = startread[i+1];

The only guard, "if (dma_q->pos + len > buf->length)", is defeated once
dma_q->pos (an int) is a few bytes into the field: pos + len wraps around
modulo 2^64 to a small value that does not exceed buf->length, so len is
left untouched and the loop writes far past the end of the VBI vb2 plane,
corrupting kernel memory. (au0828_copy_video() escapes this because it
truncates len back into a signed int that stays negative.)

An unprivileged user able to open /dev/vbiX and start streaming can reach
this path with a crafted/emulated device.

Skip packets that are too short to hold any payload once the header has
been removed.

Found by code review and reproduced under QEMU with KASAN and no hardware:
with an emulated au0828 streaming VBI, a field-start packet (so dma_q->pos
advances) followed by a 3-byte isochronous packet drives len negative and
triggers a KASAN-confirmed out-of-bounds write of the VBI vb2 plane in
au0828_copy_vbi() (inlined into au0828_isoc_copy()) in isochronous-
completion softirq context. With this patch the runt packet is dropped.

Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
Reproduced with KASAN under QEMU (no hardware): an emulated au0828 over
raw-gadget, with dummy_hcd extended to deliver isochronous packets. KASAN
reports a vmalloc out-of-bounds write in au0828_copy_vbi() (inlined into
au0828_isoc_copy()) in isochronous-completion softirq context; the
vulnerable code itself was not modified. A self-contained reproducer and
the full report are available on request.

 drivers/media/usb/au0828/au0828-video.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -599,6 +599,9 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 			dma_q->pos = 0;
 		}
 
+		if (len <= 0)
+			continue;
+
 		vbi_field_size = dev->vbi_width * dev->vbi_height * 2;
 		if (dev->vbi_read < vbi_field_size) {
 			remain  = vbi_field_size - dev->vbi_read;