[PATCH] media: ttusb-budget: check the endpoints before using them

Palla Raghunath posted 1 patch 6 days, 1 hour ago
.../media/usb/ttusb-budget/dvb-ttusb-budget.c | 45 ++++++++++++++++++-
1 file changed, 43 insertions(+), 2 deletions(-)
[PATCH] media: ttusb-budget: check the endpoints before using them
Posted by Palla Raghunath 6 days, 1 hour ago
ttusb_setup_interfaces() decides on the endpoint numbers and their
transfer types up front and never looks at what the device actually
offered:

	usb_set_interface(ttusb->dev, 1, 1);

	ttusb->bulk_out_pipe = usb_sndbulkpipe(ttusb->dev, 1);
	ttusb->bulk_in_pipe = usb_rcvbulkpipe(ttusb->dev, 1);
	ttusb->isoc_in_pipe = usb_rcvisocpipe(ttusb->dev, 2);

Give it a descriptor where endpoint 2 is an interrupt endpoint and it
still builds an isochronous pipe for it. ttusb_start_iso_xfer() later
submits an URB on that pipe with URB_ISO_ASAP set, and usb_submit_urb()
notices:

  usb 1-1: BOGUS urb xfer, pipe 0 != type 1
  usb 1-1: BOGUS urb flags, 202 --> 200
  WARNING: drivers/usb/core/urb.c:532 at usb_submit_urb+0x863/0x1870
   ttusb_start_feed+0x819/0xc20
   dmx_ts_feed_start_filtering+0xf6/0x220
   dvb_dmxdev_start_feed+0x27c/0x400
   dvb_dmxdev_filter_start+0x1b9/0xe10
   dvb_demux_do_ioctl+0xadd/0x13d0

The URB is refused, so nothing is transferred, but the warning alone is
enough to take down a machine running panic_on_warn, and attaching a USB
device is not a privileged operation.

So check first: usb_check_bulk_endpoints() covers the two bulk ones, and
since there is no isochronous equivalent the altsetting is walked here
to find the isochronous IN endpoint. Bail out with -ENODEV if either is
missing. This has to happen after usb_set_interface(), as the endpoints
we care about are in altsetting 1.

ttusb_probe() was throwing the return value away, so it needs to start
checking it too, otherwise none of the above makes any difference.

Reported-by: syzbot+3caf6a60e5f9be12de08@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3caf6a60e5f9be12de08
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
---
 .../media/usb/ttusb-budget/dvb-ttusb-budget.c | 45 ++++++++++++++++++-
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
index 7a4d28cc3242..9d6e0dfecf56 100644
--- a/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
+++ b/drivers/media/usb/ttusb-budget/dvb-ttusb-budget.c
@@ -915,7 +915,42 @@ static int ttusb_stop_feed(struct dvb_demux_feed *dvbdmxfeed)
 
 static int ttusb_setup_interfaces(struct ttusb *ttusb)
 {
-	usb_set_interface(ttusb->dev, 1, 1);
+	static const u8 bulk_ep_addrs[] = { 0x01, 0x81, 0 };
+	struct usb_host_interface *alt;
+	struct usb_interface *intf;
+	bool have_isoc_in = false;
+	int i, ret;
+
+	ret = usb_set_interface(ttusb->dev, 1, 1);
+	if (ret < 0)
+		return ret;
+
+	intf = usb_ifnum_to_if(ttusb->dev, 1);
+	if (!intf)
+		return -ENODEV;
+
+	/*
+	 * The pipes below hardcode endpoint numbers and transfer types, so
+	 * make sure the device actually has what we are about to assume it
+	 * has. Otherwise an isochronous URB ends up aimed at, say, an
+	 * interrupt endpoint and usb_submit_urb() WARNs about it.
+	 */
+	if (!usb_check_bulk_endpoints(intf, bulk_ep_addrs))
+		return -ENODEV;
+
+	/* no usb_check_isoc_endpoints() to call, so look for it by hand */
+	alt = intf->cur_altsetting;
+	for (i = 0; i < alt->desc.bNumEndpoints; i++) {
+		struct usb_endpoint_descriptor *desc = &alt->endpoint[i].desc;
+
+		if (usb_endpoint_is_isoc_in(desc) &&
+		    usb_endpoint_num(desc) == 2) {
+			have_isoc_in = true;
+			break;
+		}
+	}
+	if (!have_isoc_in)
+		return -ENODEV;
 
 	ttusb->bulk_out_pipe = usb_sndbulkpipe(ttusb->dev, 1);
 	ttusb->bulk_in_pipe = usb_rcvbulkpipe(ttusb->dev, 1);
@@ -1618,7 +1653,13 @@ static int ttusb_probe(struct usb_interface *intf, const struct usb_device_id *i
 
 	mutex_init(&ttusb->semusb);
 
-	ttusb_setup_interfaces(ttusb);
+	result = ttusb_setup_interfaces(ttusb);
+	if (result < 0) {
+		dprintk("ttusb_setup_interfaces - failed\n");
+		mutex_unlock(&ttusb->semi2c);
+		kfree(ttusb);
+		return result;
+	}
 
 	result = ttusb_alloc_iso_urbs(ttusb);
 	if (result < 0) {
-- 
2.34.1