On 21.01.2016 20:29, Joe Lawrence wrote:
On 12/23/2015 08:40 AM, Joe Lawrence wrote:
On 12/21/2015 10:07 AM, Mathias Nyman wrote:
Hi
On 18.12.2015 18:48, Joe Lawrence wrote:
Hello Roger and Mathias,
Running with slub_debug=FZPU and removing an XHCI host controller via
sysfs, I've hit a use-after-free that I've bisected to:
8c24d6d7b09deee3036ddc4f2b81b53b28c8f877 is the first bad commit
commit 8c24d6d7b09deee3036ddc4f2b81b53b28c8f877
...
If I revert 8c24d6d7b09d "usb: xhci: stop everything on the first call
to xhci_stop", the warning goes away.
Let me know if any additional instrumentation or information would help
track down this issue.
Thanks for the in-depth analysis.
Seems that the problem is that xhci driver frees data for all devices,
both
usb2 and and usb3 the first time usb_remove_hcd() is called.
All data includes the all devices all endpoints and endpoint rings,
including the
the td_list in the xhci_ring struct.
When we call usb_remove_hcd() a second time for the second xhci bus, as
part of
usb_hcd_pci_remove() in xhci_pci_remove() it will try to dequeue all
pending urbs,
but td_list is already freed for that endpoint ring.
before commit 8c24d6d7b09d we only halted xhci when first hcd was
removed,
devices were freed only after second hcd removal.
we will also free xhci->devs[slot_id] and set it to null after freeing
the endpoint rings,
so something like this should work in this case (copypaste of git diff):
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3f91270..dfc11d0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1549,7 +1549,9 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct
urb *urb, int status)
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"HW died, freeing TD.");
urb_priv = urb->hcpriv;
- for (i = urb_priv->td_cnt; i < urb_priv->length; i++) {
+ for (i = urb_priv->td_cnt;
+ i < urb_priv->length &&
xhci->devs[urb->dev->slot_id];
+ i++) {
td = urb_priv->td[i];
if (!list_empty(&td->td_list))
list_del_init(&td->td_list);
There might be other similar cases caused by commit 8c24d6d7b09d.
This whole issue probably needs more attention.
Hi Mathias,
Thanks for taking a look. I applied these changes on top of 4.3 and
didn't see any further slub_debug complaints on sysfs or hotplug removal
during overnight testing. :) Feel free to add my Reported-by or
Tested-by tag.
Gentle ping -- is there anything else I should need to test for this patch?
Somehow I missed this message earlier, sorry about that.
I'm waiting for rc-1 before I can send anything forward.
So hopefully next week
-Mathias
--
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
