Re: list_del corruption (NULL pointer dereference) on xhci-pci unbind

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 17.10.2022 21.43, Marek Marczykowski-Górecki wrote:
> On Mon, Oct 17, 2022 at 07:12:36PM +0300, Mathias Nyman wrote:
>> On 14.10.2022 23.29, Marek Marczykowski-Górecki wrote:
>>> On Fri, Oct 14, 2022 at 07:02:13PM +0300, Mathias Nyman wrote:
>>>> This whole software bandwidth issue should only be visible in Intel
>>>> Panther Point PCH xHC (Ivy bridge)
>>>
>>> It is indeed Ivy Bridge platform.
>>>
>>>> Endpoints should be deleted from bw_table list, and xhci_virt_devices
>>>> should be freed already before xhci_mem_cleanup() is called if all goes well.
>>>>
>>>> Normally endpoints are deleted from bw_table list during usb_disconnect()
>>>>
>>>> usb_disconnect()
>>>>     ...
>>>>     usb_hcd_alloc_bandwidth(dev, NULL, NULL, NULL);
>>>>       hcd->driver->drop_endpoint()  // flags endpoint to be dropped
>>>>       hcd->driver->check_bandwidth()
>>>>       ->xhci_check_bandwidth()
>>>>         xhci_configure_endpoint()
>>>>           xhci_reserve_bandwidth()  // only for Panther Point
>>>>             xhci_drop_ep_from_interval_table()
>>>>
>>>> But to avoid queuing new commands to a host in XHCI_STATE_DYING or
>>>> XHCI_STATE_REMOVING state we return early, not calling xhci_reserve_bandwidth().
>>>
>>> Indeed when I remove that early return in xhci_check_bandwidth(), the
>>> crash is gone. What's the proper solution?
>>>
>>
>> We could probably just delete the endpoint from the bw list when freeing the device and
>> endpoints. Currently we just print that "endpoint x not removed from BW list!" message
>>
>> does the below help?
> 
> Yes, this helps!

Great, thanks, I'll turn it into a proper patch.
Can I add your Reported-by and Tested-by tags to it?

> 
> xhci_drop_ep_from_interval_table() does few more things, but I assume
> this all doesn't matter at the xhci_free_virt_device() time, right?

Right, if bw_endpoint_list isn't empty when freeing the virt device
it means something prevented dropping the endpoint cleanly earlier.

Most likely host died or is being removed. We just want a clean exit

Thanks
-Mathias