Hi, On 09/28/2014 09:33 PM, Alan Stern wrote: > 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. Ah, your right, the funny thing is, checking for NOTATTACHED actually was my first plan to fix this, but when I looked at the usb_reset_and_verify_device failure path I missed the bit where it sets NOTATTACHED, so I discarded that plan, meh. I'll write a new patch implementing a check for CONFIGURED in the usb core. Regards, Hans -- 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