Re: [PATCH fix for 3.17 1/2] xhci: Check for eps[ep_index].ring being NULL after an usb_device_reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 28 Sep 2014, Hans de Goede wrote:

> >> The problem is the following call sequence (simplified):
> >>
> >> 1) usb_reset_device
> >> 2)  usb_reset_and_verify_device
> >> 2)   hub_port_init
> >> 3)    hub_port_finish_reset
> >> 3)     xhci_discover_or_reset_device
> >>         This frees xhci->devs[slot_id]->eps[ep_index].ring for all eps but 0
> >> 4)    usb_get_device_descriptor
> >>        This fails
> >> 5)   hub_port_init fails
> >> 6)  usb_reset_and_verify_device fails, does not restore device config
> >> 7)  uas_post_reset
> >> 8)   xhci_alloc_streams
> >>       NULL deref on the free-ed ring
> >>
> >> The real problem here is that xhci_discover_or_reset_device frees the rings
> >> during a reset, and if usb_reset_and_verify_device then fails to restore the
> >> device configuration (and thus re-alloc the rings), that we're then left with
> >> a device which from the usb_device_driver's pov is still functional (its
> >> disconnect method will get called when khub gets around to it), but in
> >> reality is not.
> >>
> >> This commit adds a check for this condition to xhci_check_args(), which should
> >> cover all public entry points into the xhci driver.
> > 
> > Wouldn't it be better to make usbcore check that the device state is
> > CONFIGURED (or at least, isn't NOTATTACHED) before asking the HCD to
> > allocate or release any streams?
> 
> That will not help, since khubd is the one changing the state when
> usb_reset_and_verify_device fails,

Not so.  The state is changed by:

	usb_reset_and_verify_device -> hub_port_logical_disconnect ->
		hub_port_disable -> usb_set_device_state.

This takes place without involving khubd.

>  and khubd may not yet have a chance to
> run when this happens. Also this can happen on normal urb submission after
> a (failed) reset too.

Normal URBs cannot be submitted after a failed reset, because the state 
has been changed to NOTATTACHED.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]