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 --- .../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