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 linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
