[PATCH 0/3] media: USB: validate transfer lengths in URB callbacks

[PATCH 0/3] media: USB: validate transfer lengths in URB callbacks

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

Several media USB drivers process URB transfer buffers at fixed
offsets without checking actual_length or iso_frame_desc
actual_length.  When a device delivers a short transfer, the
arithmetic on the length value wraps and the driver reads or copies
from an unintended memory offset.

This series adds minimum length checks to three drivers:

 - cx231xx video: isoc and bulk handlers need buffer_size >= 4
   before computing p_buffer + buffer_size - 4
 - cx231xx VBI: same pattern, raise threshold from > 0 to >= 4
 - au0828: isoc handler subtracts 4-byte header, raise threshold
   from > 0 to >= 5

Tristan Madani (3):
  media: cx231xx: validate minimum transfer length in video handlers
  media: cx231xx: require minimum VBI transfer length
  media: au0828: require minimum isochronous packet length

 drivers/media/usb/au0828/au0828-video.c     | 2 +-
 drivers/media/usb/cx231xx/cx231xx-vbi.c     | 2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c   | 6 ++++++
 3 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.39.5

[PATCH 1/3] media: cx231xx: validate minimum transfer length in video handlers

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

cx231xx_isoc_copy() and cx231xx_bulk_copy() both save the last 4
bytes of the transfer buffer via memcpy(dma_q->partial_buf,
p_buffer + buffer_size - 4, 4).  When the transfer length is less
than 4, the subtraction wraps to a large value and the memcpy reads
from an unintended memory offset.

Add a minimum length check to both handlers before processing.

Fixes: 64fbf4445526 ("[media] cx231xx: Added support for Carraera, Shelby, RDx_253S and VIDEO_GRABBER")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/media/usb/cx231xx/cx231xx-video.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 70aa99fead278..76776cd6e5691 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -303,6 +303,9 @@ static inline int cx231xx_isoc_copy(struct cx231xx *dev, struct urb *urb)
 		buffer_size = urb->iso_frame_desc[i].actual_length;
 		bytes_parsed = 0;
 
+		if (buffer_size < 4)
+			continue;
+
 		if (dma_q->is_partial_line) {
 			/* Handle the case of a partial line */
 			sav_eav = dma_q->last_sav;
@@ -382,6 +385,9 @@ static inline int cx231xx_bulk_copy(struct cx231xx *dev, struct urb *urb)
 		buffer_size = urb->actual_length;
 		bytes_parsed = 0;
 
+		if (buffer_size < 4)
+			return 0;
+
 		if (dma_q->is_partial_line) {
 			/* Handle the case of a partial line */
 			sav_eav = dma_q->last_sav;
-- 
2.47.3

[PATCH 2/3] media: cx231xx: require minimum VBI transfer length

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

cx231xx_isoc_vbi_copy() enters its processing loop when
buffer_size > 0 but then executes memcpy(dma_q->partial_buf,
p_buffer + buffer_size - 4, 4) which needs at least 4 bytes.
When buffer_size is 1-3, the subtraction wraps and the memcpy
reads from an unintended offset.

Raise the minimum from > 0 to >= 4.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/media/usb/cx231xx/cx231xx-vbi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-vbi.c b/drivers/media/usb/cx231xx/cx231xx-vbi.c
index 338e101484653..c019649c62210 100644
--- a/drivers/media/usb/cx231xx/cx231xx-vbi.c
+++ b/drivers/media/usb/cx231xx/cx231xx-vbi.c
@@ -92,7 +92,7 @@ static inline int cx231xx_isoc_vbi_copy(struct cx231xx *dev, struct urb *urb)
 	p_buffer = urb->transfer_buffer;
 	buffer_size = urb->actual_length;
 
-	if (buffer_size > 0) {
+	if (buffer_size >= 4) {
 		bytes_parsed = 0;
 
 		if (dma_q->is_partial_line) {
-- 
2.47.3

[PATCH 3/3] media: au0828: require minimum isochronous packet length

[Tristan Madani]

From: Tristan Madani <tristan@talencesecurity.com>

au0828_isoc_copy() subtracts 4 from actual_length to skip the
packet header: len = urb->iso_frame_desc[i].actual_length - 4.
The existing check rejects only actual_length <= 0, so a length
of 1-3 causes len to wrap negative.  The wrapped value is then
passed to the copy helpers as an unsigned size.

Raise the minimum from > 0 to >= 5 (4-byte header plus at least
one byte of payload).

Fixes: 8b2f07952345 ("V4L/DVB (11066): au0828: add support for analog functionality in bridge")
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 drivers/media/usb/au0828/au0828-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 3c53105f3d2b3..6cc99d1a2fbce 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -530,7 +530,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 				continue;
 		}
 
-		if (urb->iso_frame_desc[i].actual_length <= 0)
+		if (urb->iso_frame_desc[i].actual_length < 5)
 			continue;
 
 		if (urb->iso_frame_desc[i].actual_length >
-- 
2.47.3