On Fri, Apr 1, 2016 at 8:00 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > On Fri, 1 Apr 2016, Navin P.S wrote: > >> Hi, >> >> I was looking at the bug https://bugzilla.kernel.org/show_bug.cgi?id=112171 >> which says >> >> Feb 08 22:58:56 x kernel: UBSAN: Undefined behaviour in >> drivers/usb/host/ehci-hub.c:873:47 >> Feb 08 22:58:56 x kernel: index -1 is out of range for type 'u32 [1]' >> >> >> >> I'm suspective the ehci-tegra function ehci-tegra.c:291 calling with >> wIndex as 0 . >> >> I'm not sure if that is even possible .So i ask in this list ? Can >> someone give pointers ? > > Yes, it is possible. tegra_ehci_hub_control() needs to verify that > wIndex > 0 before using status_reg. Would you like to write a patch to > fix this? > The actual bug submitted looks to be in a different call stack as i've analzyed below but this diff also fixes tegra_ehci_hub_control if it is called with wIndex 0. If you can confirm below i'll copy lkml after the submitter verifies. In case of wIndex can we return -EPIPE as in case of error in ehci-hub.c or do we have to return some other error code ? diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c index ffc9029..50194af 100644 --- a/drivers/usb/host/ehci-hub.c +++ b/drivers/usb/host/ehci-hub.c @@ -872,6 +872,10 @@ int ehci_hub_control( ) { struct ehci_hcd *ehci = hcd_to_ehci (hcd); int ports = HCS_N_PORTS (ehci->hcs_params); + + if (!wIndex || wIndex > ports) + return -EPIPE; + In the below call trace we get wIndex as 0 due to usb_set_configuration calling it with 0. Please see below. Feb 08 22:58:56 x kernel: UBSAN: Undefined behaviour in drivers/usb/host/ehci-hub.c:873:47 Feb 08 22:58:56 x kernel: index -1 is out of range for type 'u32 [1]' Feb 08 22:58:56 x kernel: Call Trace: Feb 08 22:58:56 x kernel: [<ffffffff81832ca4>] dump_stack+0xaf/0x10c Feb 08 22:58:56 x kernel: [<ffffffff818a6360>] ubsan_epilogue+0x14/0x56 Feb 08 22:58:56 x kernel: [<ffffffff818a6c47>] __ubsan_handle_out_of_bounds+0x86/0xb3 Feb 08 22:58:56 x kernel: [<ffffffff81e65a88>] ehci_hub_control+0xcf/0x141e Feb 08 22:58:56 x kernel: [<ffffffff81e35a24>] rh_call_control+0xaa6/0xcf1 Feb 08 22:58:56 x kernel: [<ffffffff81180000>] ? hib_wait_io+0x64/0x132 Feb 08 22:58:56 x kernel: [<ffffffff81d0207d>] ? dev_vprintk_emit+0x391/0x3be Feb 08 22:58:56 x kernel: [<ffffffff81e3701b>] usb_hcd_submit_urb+0x318/0x592 Feb 08 22:58:56 x kernel: [<ffffffff81e3a0fc>] usb_submit_urb+0xc26/0xc50 Feb 08 22:58:56 x kernel: [<ffffffff81e3c134>] usb_start_wait_urb+0xb1/0x198 Feb 08 22:58:56 x kernel: [<ffffffff81e390f2>] ? usb_alloc_urb+0x21/0x6a Feb 08 22:58:56 x kernel: [<ffffffff81e3c714>] usb_control_msg+0x1c5/0x21d Feb 08 22:58:56 x kernel: [<ffffffff81e2f635>] hub_probe+0x8ed/0x18cb Feb 08 22:58:56 x kernel: [<ffffffff81129fbb>] ? get_parent_ip+0x14/0x6d Feb 08 22:58:56 x kernel: [<ffffffff81e43d0d>] usb_probe_interface+0x362/0x48e Feb 08 22:58:56 x kernel: [<ffffffff81d0aa47>] driver_probe_device+0x392/0x719 Feb 08 22:58:56 x kernel: [<ffffffff81d0b546>] __device_attach_driver+0x1bd/0x1cf Feb 08 22:58:56 x kernel: [<ffffffff81d0b389>] ? device_release_driver+0x4c/0x4c Feb 08 22:58:56 x kernel: [<ffffffff81d06721>] bus_for_each_drv+0x136/0x165 Feb 08 22:58:56 x kernel: [<ffffffff81d0a56b>] __device_attach+0x148/0x23a Feb 08 22:58:56 x kernel: [<ffffffff81d0b572>] device_initial_probe+0x1a/0x23 Feb 08 22:58:56 x kernel: [<ffffffff81d089e2>] bus_probe_device+0x98/0x1bc Feb 08 22:58:56 x kernel: [<ffffffff81d03f8e>] device_add+0x914/0xc99 Feb 08 22:58:56 x kernel: [<ffffffff81e3eca0>] usb_set_configuration+0xee2/0xfc8 core/message.c Here we set wIndex = 0 and it propogates down Function usb_set_configuration 880 ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), 1881 USB_REQ_SET_CONFIGURATION, 0, configuration, 0, 1882 NULL, 0, USB_CTRL_SET_TIMEOUT); calls --> Function int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request, __u8 requesttype, __u16 value, __u16 index, void *data, __u16 size, int timeout) 145 dr->wIndex = cpu_to_le16(index); 146 dr->wLength = cpu_to_le16(size); 147 148 ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout); 149 Concern: index is 0 calls---> 79 /* returns status (negative) or length (positive) */ 80 static int usb_internal_control_msg(struct usb_device *usb_dev, 81 unsigned int pipe, 82 struct usb_ctrlrequest *cmd, 96 retv = usb_start_wait_urb(urb, timeout, &length); static int usb_start_wait_urb(struct urb *urb, int timeout, int *actual_length) usb_submit_urb in usb/core/urb.c calls int usb_submit_urb(struct urb *urb, gfp_t mem_flags) calls return usb_hcd_submit_urb(urb, mem_flags); in usb/core/hcd.c int usb_hcd_submit_urb (struct urb *urb, gfp_t mem_flags) calls rh_call_control static int rh_call_control (struct usb_hcd *hcd, struct urb *urb) { struct usb_ctrlrequest *cmd; might_sleep(); spin_lock_irq(&hcd_root_hub_lock); status = usb_hcd_link_urb_to_ep(hcd, urb); spin_unlock_irq(&hcd_root_hub_lock); if (status) return status; urb->hcpriv = hcd; /* Indicate it's queued */ cmd = (struct usb_ctrlrequest *) urb->setup_packet; typeReq = (cmd->bRequestType << 8) | cmd->bRequest; wValue = le16_to_cpu (cmd->wValue); wIndex = le16_to_cpu (cmd->wIndex); wLength = le16_to_cpu (cmd->wLength); calls status = hcd->driver->hub_control (hcd, typeReq, wValue, wIndex, tbuf, wLength); wIndex is 0 and we get ehci_hub_control error for negative index. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
