fix missing endpoint sanity checks

PATCH In review Johan Hovold 1 participant 8 messages Mar 13, 2017 – Apr 3, 2017

Subsystems (3)

Stable branch Media (V4L/DVB) Gspca USB webcam driver

lore

Share & tools

lore

Johan Hovold

author

2017-03-13 12:54 UTC #0 lore raw

cover fix missing endpoint sanity checks

This series fixes a number of NULL-pointer dereferences (and related issues) due to missing endpoint sanity checks that can be triggered by a malicious USB device.

Johan

Johan Hovold (6):
[media] dib0700: fix NULL-deref at probe [media] usbvision: fix NULL-deref at probe [media] cx231xx-cards: fix NULL-deref at probe [media] cx231xx-audio: fix init error path [media] cx231xx-audio: fix NULL-deref at probe [media] gspca: konica: add missing endpoint sanity check

 drivers/media/usb/cx231xx/cx231xx-audio.c     | 42 +++++++++++++++++--------
 drivers/media/usb/cx231xx/cx231xx-cards.c     | 45 ++++++++++++++++++++++++---
 drivers/media/usb/dvb-usb/dib0700_core.c      |  3 ++
 drivers/media/usb/gspca/konica.c              |  3 ++
 drivers/media/usb/usbvision/usbvision-video.c |  9 +++++-
 5 files changed, 83 insertions(+), 19 deletions(-)

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:54 UTC #1 lore raw

PATCH 1/6 dib0700: fix NULL-deref at probe

Make sure to check the number of endpoints to avoid dereferencing a NULL-pointer should a malicious device lack endpoints.

Fixes c4018fa2e4c0 ("[media] dib0700: fix RC support on Hauppauge

Nova-TD")

Cc stable <[email protected]> # 3.16

Cc Mauro Carvalho Chehab <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

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

drivers/media/usb/dvb-usb/dib0700_core.c +3 -0

index dd5edd3a17ee..08acdd32e412 100644
@@ -809,6 +809,9 @@ int dib0700_rc_setup(struct dvb_usb_device *d, struct usb_interface *intf)
 
 	/* Starting in firmware 1.20, the RC info is provided on a bulk pipe */
 
+	if (intf->altsetting[0].desc.bNumEndpoints < rc_ep + 1)
+		return -ENODEV;
+
 	purb = usb_alloc_urb(0, GFP_KERNEL);
 	if (purb == NULL)
 		return -ENOMEM;

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:54 UTC #2 lore raw

PATCH 2/6 usbvision: fix NULL-deref at probe

In reply to Johan Hovold

Make sure to check the number of endpoints to avoid dereferencing a NULL-pointer or accessing memory beyond the endpoint array should a malicious device lack the expected endpoints.

Fixes 2a9f8b5d25be ("V4L/DVB (5206): Usbvision: set alternate interface

modification")

Cc stable <[email protected]> # 2.6.21

Cc Thierry MERLE <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/media/usb/usbvision/usbvision-video.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

drivers/media/usb/usbvision/usbvision-video.c +8 -1

index f5c635a67d74..f9c3325aa4d4 100644
@@ -1501,7 +1501,14 @@ static int usbvision_probe(struct usb_interface *intf,
 	}
 
 	for (i = 0; i < usbvision->num_alt; i++) {
-		u16 tmp = le16_to_cpu(uif->altsetting[i].endpoint[1].desc.
+		u16 tmp;
+
+		if (uif->altsetting[i].desc.bNumEndpoints < 2) {
+			ret = -ENODEV;
+			goto err_pkt;
+		}
+
+		tmp = le16_to_cpu(uif->altsetting[i].endpoint[1].desc.
 				      wMaxPacketSize);
 		usbvision->alt_max_pkt_size[i] =
 			(tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:54 UTC #3 lore raw

PATCH 3/6 cx231xx-cards: fix NULL-deref at probe

In reply to Johan Hovold

Make sure to check the number of endpoints to avoid dereferencing a NULL-pointer or accessing memory beyond the endpoint array should a malicious device lack the expected endpoints.

Fixes e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")

Cc stable <[email protected]> # 2.6.30

Cc Sri Deevi <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/media/usb/cx231xx/cx231xx-cards.c | 45 +++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 5 deletions(-)

drivers/media/usb/cx231xx/cx231xx-cards.c +40 -5

index f730fdbc9156..f850267a0095 100644
@@ -1426,6 +1426,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 
 	uif = udev->actconfig->interface[idx];
 
+	if (uif->altsetting[0].desc.bNumEndpoints < isoc_pipe + 1)
+		return -ENODEV;
+
 	dev->video_mode.end_point_addr = uif->altsetting[0].endpoint[isoc_pipe].desc.bEndpointAddress;
 	dev->video_mode.num_alt = uif->num_altsetting;

@@ -1439,7 +1442,12 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		return -ENOMEM;
 
 	for (i = 0; i < dev->video_mode.num_alt; i++) {
-		u16 tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].desc.wMaxPacketSize);
+		u16 tmp;
+
+		if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1)
+			return -ENODEV;
+
+		tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].desc.wMaxPacketSize);
 		dev->video_mode.alt_max_pkt_size[i] = (tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);
 		dev_dbg(dev->dev,
 			"Alternate setting %i, max size= %i\n", i,

@@ -1456,6 +1464,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 	}
 	uif = udev->actconfig->interface[idx];
 
+	if (uif->altsetting[0].desc.bNumEndpoints < isoc_pipe + 1)
+		return -ENODEV;
+
 	dev->vbi_mode.end_point_addr =
 	    uif->altsetting[0].endpoint[isoc_pipe].desc.
 			bEndpointAddress;

@@ -1472,8 +1483,12 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		return -ENOMEM;
 
 	for (i = 0; i < dev->vbi_mode.num_alt; i++) {
-		u16 tmp =
-		    le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].
+		u16 tmp;
+
+		if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1)
+			return -ENODEV;
+
+		tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].
 				desc.wMaxPacketSize);
 		dev->vbi_mode.alt_max_pkt_size[i] =
 		    (tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);

@@ -1493,6 +1508,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 	}
 	uif = udev->actconfig->interface[idx];
 
+	if (uif->altsetting[0].desc.bNumEndpoints < isoc_pipe + 1)
+		return -ENODEV;
+
 	dev->sliced_cc_mode.end_point_addr =
 	    uif->altsetting[0].endpoint[isoc_pipe].desc.
 			bEndpointAddress;

@@ -1507,7 +1525,12 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		return -ENOMEM;
 
 	for (i = 0; i < dev->sliced_cc_mode.num_alt; i++) {
-		u16 tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].
+		u16 tmp;
+
+		if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1)
+			return -ENODEV;
+
+		tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].
 				desc.wMaxPacketSize);
 		dev->sliced_cc_mode.alt_max_pkt_size[i] =
 		    (tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);

@@ -1676,6 +1699,11 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 		}
 		uif = udev->actconfig->interface[idx];
 
+		if (uif->altsetting[0].desc.bNumEndpoints < isoc_pipe + 1) {
+			retval = -ENODEV;
+			goto err_video_alt;
+		}
+
 		dev->ts1_mode.end_point_addr =
 		    uif->altsetting[0].endpoint[isoc_pipe].
 				desc.bEndpointAddress;

@@ -1693,7 +1721,14 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 		}
 
 		for (i = 0; i < dev->ts1_mode.num_alt; i++) {
-			u16 tmp = le16_to_cpu(uif->altsetting[i].
+			u16 tmp;
+
+			if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1) {
+				retval = -ENODEV;
+				goto err_video_alt;
+			}
+
+			tmp = le16_to_cpu(uif->altsetting[i].
 						endpoint[isoc_pipe].desc.
 						wMaxPacketSize);
 			dev->ts1_mode.alt_max_pkt_size[i] =

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:54 UTC #4 lore raw

PATCH 4/6 cx231xx-audio: fix init error path

In reply to Johan Hovold

Make sure to release the snd_card also on a late allocation error.

Fixes e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")

Cc stable <[email protected]> # 2.6.30

Cc Sri Deevi <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/media/usb/cx231xx/cx231xx-audio.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

drivers/media/usb/cx231xx/cx231xx-audio.c +14 ~~-11~~

index cf80842dfa08..f3729d6eb46a 100644
@@ -670,10 +670,8 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 	spin_lock_init(&adev->slock);
 	err = snd_pcm_new(card, "Cx231xx Audio", 0, 0, 1, &pcm);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto err_free_card;
 
 	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE,
 			&snd_cx231xx_pcm_capture);

@@ -687,10 +685,9 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	INIT_WORK(&dev->wq_trigger, audio_trigger);
 
 	err = snd_card_register(card);
-	if (err < 0) {
-		snd_card_free(card);
-		return err;
-	}
+	if (err < 0)
+		goto err_free_card;
+
 	adev->sndcard = card;
 	adev->udev = dev->udev;

@@ -709,9 +706,10 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 		"audio EndPoint Addr 0x%x, Alternate settings: %i\n",
 		adev->end_point_addr, adev->num_alt);
 	adev->alt_max_pkt_size = kmalloc(32 * adev->num_alt, GFP_KERNEL);
-
-	if (adev->alt_max_pkt_size == NULL)
-		return -ENOMEM;
+	if (!adev->alt_max_pkt_size) {
+		err = -ENOMEM;
+		goto err_free_card;
+	}
 
 	for (i = 0; i < adev->num_alt; i++) {
 		u16 tmp =

@@ -725,6 +723,11 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	}
 
 	return 0;
+
+err_free_card:
+	snd_card_free(card);
+
+	return err;
 }
 
 static int cx231xx_audio_fini(struct cx231xx *dev)

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:54 UTC #5 lore raw

PATCH 5/6 cx231xx-audio: fix NULL-deref at probe

In reply to Johan Hovold

Make sure to check the number of endpoints to avoid dereferencing a NULL-pointer or accessing memory beyond the endpoint array should a malicious device lack the expected endpoints.

Fixes e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")

Cc stable <[email protected]> # 2.6.30

Cc Sri Deevi <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

---
 drivers/media/usb/cx231xx/cx231xx-audio.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

drivers/media/usb/cx231xx/cx231xx-audio.c +15 -2

index f3729d6eb46a..a050d125934c 100644
@@ -697,6 +697,11 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 					    hs_config_info[0].interface_info.
 					    audio_index + 1];
 
+	if (uif->altsetting[0].desc.bNumEndpoints < isoc_pipe + 1) {
+		err = -ENODEV;
+		goto err_free_card;
+	}
+
 	adev->end_point_addr =
 	    uif->altsetting[0].endpoint[isoc_pipe].desc.
 			bEndpointAddress;

@@ -712,8 +717,14 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	}
 
 	for (i = 0; i < adev->num_alt; i++) {
-		u16 tmp =
-		    le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].desc.
+		u16 tmp;
+
+		if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1) {
+			err = -ENODEV;
+			goto err_free_pkt_size;
+		}
+
+		tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].desc.
 				wMaxPacketSize);
 		adev->alt_max_pkt_size[i] =
 		    (tmp & 0x07ff) * (((tmp & 0x1800) >> 11) + 1);

@@ -724,6 +735,8 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 	return 0;
 
+err_free_pkt_size:
+	kfree(adev->alt_max_pkt_size);
 err_free_card:
 	snd_card_free(card);

Signature · 1 line

2.12.0

Johan Hovold

author

2017-03-13 12:53 UTC #6 lore raw

PATCH 6/6 gspca: konica: add missing endpoint sanity check

In reply to Johan Hovold

Make sure to check the number of endpoints to avoid accessing memory beyond the endpoint array should a device lack the expected endpoints.

Note that, as far as I can tell, the gspca framework has already made sure there is at least one endpoint in the current alternate setting so there should be no risk for a NULL-pointer dereference here.

Fixes b517af722860 ("V4L/DVB: gspca_konica: New gspca subdriver for

konica chipset using cams")

Cc stable <[email protected]> # 2.6.37

Cc Hans de Goede <[email protected]>

Signed-off-by Johan Hovold <[email protected]>

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

drivers/media/usb/gspca/konica.c +3 -0

index 71f273377f83..31b2117e8f1d 100644
@@ -184,6 +184,9 @@ static int sd_start(struct gspca_dev *gspca_dev)
 		return -EIO;
 	}
 
+	if (alt->desc.bNumEndpoints < 2)
+		return -ENODEV;
+
 	packet_size = le16_to_cpu(alt->endpoint[0].desc.wMaxPacketSize);
 
 	n = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].priv;

Signature · 1 line

2.12.0

Johan Hovold

author

2017-04-03 08:30 UTC #7 lore raw

In reply to Johan Hovold

Quoted 11 lines · Johan Hovold

This series fixes a number of NULL-pointer dereferences (and related issues) due to missing endpoint sanity checks that can be triggered by a malicious USB device.
Johan Hovold (6):
[media] dib0700: fix NULL-deref at probe [media] usbvision: fix NULL-deref at probe [media] cx231xx-cards: fix NULL-deref at probe [media] cx231xx-audio: fix init error path [media] cx231xx-audio: fix NULL-deref at probe [media] gspca: konica: add missing endpoint sanity check

I noticed these had been assigned to you, Hans. Anything more you need to get them merged?

Thanks,
Johan