On Wed, 16 Jan 2013, Martin Mokrejs wrote:

> A corresponding diff of dmesg output is attached. Note that the first 
> kmemleak in there
> happened just without any prior fiddling with a USB drive. For about two days 
> I haven't
> connected a drive. However, usb-storage might be in a wrong shape since 
> several days
> when it happened for the first time. Did you want me to reboot? ;-) I did not.

Sarah, I looked through the xhci-hcd driver.  There does appear to be a
leak in xhci-ring.c:handle_tx_event().  The routine looks like this:

                        /* Leave the TD around for the reset endpoint function
                         * to use(but only if it's not a control endpoint,
                         * since we already queued the Set TR dequeue pointer
                         * command for stalled control endpoints).
                         */
                        if (usb_endpoint_xfer_control(&urb->ep->desc) ||
                                (trb_comp_code != COMP_STALL &&
                                        trb_comp_code != COMP_BABBLE))
                                xhci_urb_free_priv(xhci, urb_priv);

                        ...

                        /* EHCI, UHCI, and OHCI always unconditionally set the
                         * urb->status of an isochronous endpoint to 0.
                         */
                        if (usb_pipetype(urb->pipe) == PIPE_ISOCHRONOUS)
                                status = 0;
                        usb_hcd_giveback_urb(bus_to_hcd(urb->dev->bus), urb, 
status);

If the condition on the first "if" statement fails, urb_priv won't be 
deallocated.  It needs something like

                        if (...)
                                xhci_urb_free_priv(xhci, urb_priv);
+                       else
+                               kfree(urb_priv);

Martin, can you tell if adding these two lines fixes the problem?

Also, the comment is wrong.  urb->status is not set to 0
unconditionally for isochonrous endpoints in [eou]hcd-hcd.  If any of
the iso packets gets a nonzero status then urb->status is set to one of
those values, not to 0.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to