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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
