diff options
author | Arvid Brodin <arvid.brodin@enea.com> | 2011-11-23 18:13:02 +0100 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2011-11-26 20:02:47 -0800 |
commit | c64391f264b7658c00515173cca58f5b054af1a2 (patch) | |
tree | 67a403acfca8903a10bd45b86fe22488de476472 | |
parent | e08f6a2790aaf3563e7800399321c0fb9a6c6636 (diff) | |
download | linux-c64391f264b7658c00515173cca58f5b054af1a2.tar.bz2 |
usb/isp1760: Fix race condition memory leak
This fixes a memory leak reported by Catalin Marinas:
schedule_ptds() is called from isp1760_irq() and removes the qh from the
controlqhs queue but ep->hcpriv still points to the qh and therefore it is not
freed.
Shortly after this, the isp1760_endpoint_disable() function sets ep->hcpriv to
NULL and calls schedule_ptds() but since the corresponding qh is no longer in
the queue, it is simply forgotten and reported by kmemleak.
With this patch, the qh is always freed at endpoint_disable, instead, and the
corresponding entry removed from the queue head list.
While I was at it, I also replaced the lines in isp1760_endpoint_disable()
that removed remaining qtds from the qh with a WARN_ON check for non-empty qh,
in line with earlier comments from Alan Stern (linux-usb list, 2011-07-20).
Signed-off-by: Arvid Brodin <arvid.brodin@enea.com>
Tested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/usb/host/isp1760-hcd.c | 30 |
1 files changed, 12 insertions, 18 deletions
diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c index a760fbf18ebe..fc72d44bf787 100644 --- a/drivers/usb/host/isp1760-hcd.c +++ b/drivers/usb/host/isp1760-hcd.c @@ -937,7 +937,6 @@ void schedule_ptds(struct usb_hcd *hcd) struct isp1760_hcd *priv; struct isp1760_qh *qh, *qh_next; struct list_head *ep_queue; - struct usb_host_endpoint *ep; LIST_HEAD(urb_list); struct urb_listitem *urb_listitem, *urb_listitem_next; int i; @@ -955,17 +954,9 @@ void schedule_ptds(struct usb_hcd *hcd) for (i = 0; i < QH_END; i++) { ep_queue = &priv->qh_list[i]; list_for_each_entry_safe(qh, qh_next, ep_queue, qh_list) { - ep = list_entry(qh->qtd_list.next, struct isp1760_qtd, - qtd_list)->urb->ep; collect_qtds(hcd, qh, &urb_list); - if (list_empty(&qh->qtd_list)) { + if (list_empty(&qh->qtd_list)) list_del(&qh->qh_list); - if (ep->hcpriv == NULL) { - /* Endpoint has been disabled, so we - can free the associated queue head. */ - qh_free(qh); - } - } } } @@ -1708,8 +1699,8 @@ static void isp1760_endpoint_disable(struct usb_hcd *hcd, { struct isp1760_hcd *priv = hcd_to_priv(hcd); unsigned long spinflags; - struct isp1760_qh *qh; - struct isp1760_qtd *qtd; + struct isp1760_qh *qh, *qh_iter; + int i; spin_lock_irqsave(&priv->lock, spinflags); @@ -1717,14 +1708,17 @@ static void isp1760_endpoint_disable(struct usb_hcd *hcd, if (!qh) goto out; - list_for_each_entry(qtd, &qh->qtd_list, qtd_list) - if (qtd->status != QTD_RETIRE) { - dequeue_urb_from_qtd(hcd, qh, qtd); - qtd->urb->status = -ECONNRESET; - } + WARN_ON(!list_empty(&qh->qtd_list)); + for (i = 0; i < QH_END; i++) + list_for_each_entry(qh_iter, &priv->qh_list[i], qh_list) + if (qh_iter == qh) { + list_del(&qh_iter->qh_list); + i = QH_END; + break; + } + qh_free(qh); ep->hcpriv = NULL; - /* Cannot free qh here since it will be parsed by schedule_ptds() */ schedule_ptds(hcd); |