linux-kernel - [PATCH v3] media: uvcvideo: read bulk URBs after maxPayloadSize

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Mon, 20 Apr 2020 12:15:06 -0700
From:   Julian Meyer <julianmeyer2000@...il.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     Julian Meyer <julianmeyer2000@...il.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v3] media: uvcvideo: read bulk URBs after maxPayloadSize

This fixes a bug that caused certain Realtek cameras to crash.

The camera would send additional UVC payloads after the maxPayloadSize
was reached. This patch modifies uvc_video_decode_bulk such that it
continues reading payloads when it reaches the maxPayloadSize if there
is more data left.

There are also various safeguards included to prevent the loop
from running forever. We'll try to start reading if there is
more data, but if we can't decode any more data, we exit the loop.

Signed-off-by: Julian Meyer <julianmeyer2000@...il.com>
---
 drivers/media/usb/uvc/uvc_video.c | 85 +++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 22 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 8fa77a81dd7f..5e1a6ce37724 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1374,31 +1374,24 @@ static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
 	}
 }
 
-static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
-			struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
+static int uvc_video_decode_bulk_single(struct uvc_streaming *stream,
+	struct uvc_buffer *buf, struct uvc_buffer *meta_buf,
+	struct uvc_urb *uvc_urb, u8 **mem, int *len)
 {
-	struct urb *urb = uvc_urb->urb;
-	struct uvc_streaming *stream = uvc_urb->stream;
-	u8 *mem;
-	int len, ret;
+	unsigned int bytes_left;
+	int ret;
 
-	/*
-	 * Ignore ZLPs if they're not part of a frame, otherwise process them
-	 * to trigger the end of payload detection.
-	 */
-	if (urb->actual_length == 0 && stream->bulk.header_size == 0)
-		return;
+	int length_removed = 0;
 
-	mem = urb->transfer_buffer;
-	len = urb->actual_length;
-	stream->bulk.payload_size += len;
+	struct urb *urb = uvc_urb->urb;
+	unsigned int max_size = stream->bulk.max_payload_size;
 
 	/* If the URB is the first of its payload, decode and save the
 	 * header.
 	 */
 	if (stream->bulk.header_size == 0 && !stream->bulk.skip_payload) {
 		do {
-			ret = uvc_video_decode_start(stream, buf, mem, len);
+			ret = uvc_video_decode_start(stream, buf, *mem, *len);
 			if (ret == -EAGAIN)
 				uvc_video_next_buffers(stream, &buf, &meta_buf);
 		} while (ret == -EAGAIN);
@@ -1407,13 +1400,15 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
 		if (ret < 0 || buf == NULL) {
 			stream->bulk.skip_payload = 1;
 		} else {
-			memcpy(stream->bulk.header, mem, ret);
+			memcpy(stream->bulk.header, *mem, ret);
 			stream->bulk.header_size = ret;
 
-			uvc_video_decode_meta(stream, meta_buf, mem, ret);
+			uvc_video_decode_meta(stream, meta_buf, *mem, ret);
 
-			mem += ret;
-			len -= ret;
+			*mem += ret;
+			length_removed += ret;
+			*len -= ret;
+			stream->bulk.payload_size += ret;
 		}
 	}
 
@@ -1423,8 +1418,22 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
 	 */
 
 	/* Prepare video data for processing. */
-	if (!stream->bulk.skip_payload && buf != NULL)
-		uvc_video_decode_data(uvc_urb, buf, mem, len);
+	if (!stream->bulk.skip_payload && buf != NULL) {
+		bytes_left = min((unsigned int) *len,
+			max_size - stream->bulk.payload_size);
+
+		stream->bulk.payload_size += bytes_left;
+
+		uvc_video_decode_data(uvc_urb, buf, *mem, bytes_left);
+
+		*len -= bytes_left;
+		*mem += bytes_left;
+		length_removed = bytes_left;
+	} else {
+		stream->bulk.payload_size += *len;
+		length_removed = *len;
+		*len = 0;
+	}
 
 	/* Detect the payload end by a URB smaller than the maximum size (or
 	 * a payload size equal to the maximum) and process the header again.
@@ -1442,6 +1451,38 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
 		stream->bulk.skip_payload = 0;
 		stream->bulk.payload_size = 0;
 	}
+
+	return length_removed;
+}
+
+static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
+			struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
+{
+	struct urb *urb = uvc_urb->urb;
+	struct uvc_streaming *stream = uvc_urb->stream;
+	u8 *mem;
+	int len;
+	int len_processed;
+
+	/*
+	 * Ignore ZLPs if they're not part of a frame, otherwise process them
+	 * to trigger the end of payload detection.
+	 */
+	if (urb->actual_length == 0 && stream->bulk.header_size == 0)
+		return;
+
+	mem = urb->transfer_buffer;
+	len = urb->actual_length;
+
+	while (len > 0) {
+		len_processed = uvc_video_decode_bulk_single(stream, buf,
+			meta_buf, uvc_urb, &mem, &len);
+
+		// if we don't process anything, we break out of the decode loop
+		if (len_processed == 0) {
+			return;
+		}
+	}
 }
 
 static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb,
-- 
2.26.1