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]

 



Hi,

On 09/28/2014 04:29 PM, Alan Stern wrote:
> On Sat, 27 Sep 2014, Hans de Goede wrote:
> 
>> This commit fixes the following oops:
>>
>> [10238.622067] scsi host3: uas_eh_bus_reset_handler start
>> [10240.766164] usb 3-4: reset SuperSpeed USB device number 3 using xhci_hcd
>> [10245.779365] usb 3-4: device descriptor read/8, error -110
>> [10245.883331] usb 3-4: reset SuperSpeed USB device number 3 using xhci_hcd
>> [10250.897603] usb 3-4: device descriptor read/8, error -110
>> [10251.058200] BUG: unable to handle kernel NULL pointer dereference at 0000000000000040
>> [10251.058244] IP: [<ffffffff815ac6e1>] xhci_check_streams_endpoint+0x91/0x140
>> <snip>
>> [10251.059473] Call Trace:
>> [10251.059487]  [<ffffffff815aca6c>] xhci_calculate_streams_and_bitmask+0xbc/0x130
>> [10251.059520]  [<ffffffff815aeb5f>] xhci_alloc_streams+0x10f/0x5a0
>> [10251.059548]  [<ffffffff810a4685>] ? check_preempt_curr+0x75/0xa0
>> [10251.059575]  [<ffffffff810a46dc>] ? ttwu_do_wakeup+0x2c/0x100
>> [10251.059601]  [<ffffffff810a49e6>] ? ttwu_do_activate.constprop.111+0x66/0x70
>> [10251.059635]  [<ffffffff815779ab>] usb_alloc_streams+0xab/0xf0
>> [10251.059662]  [<ffffffffc0616b48>] uas_configure_endpoints+0x128/0x150 [uas]
>> [10251.059694]  [<ffffffffc0616bac>] uas_post_reset+0x3c/0xb0 [uas]
>> [10251.059722]  [<ffffffff815727d9>] usb_reset_device+0x1b9/0x2a0
>> [10251.059749]  [<ffffffffc0616f42>] uas_eh_bus_reset_handler+0xb2/0x190 [uas]
>> [10251.059781]  [<ffffffff81514293>] scsi_try_bus_reset+0x53/0x110
>> [10251.059808]  [<ffffffff815163b7>] scsi_eh_bus_reset+0xf7/0x270
>> <snip>
>>
>> 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, 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.

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




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