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@xxxxxxxx>
Tested-by: Catalin Marinas <catalin.marinas@xxxxxxx>
---
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 1adb21e..2df6631 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -922,7 +922,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;
@@ -940,17 +939,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);
- }
- }
}
}
@@ -1692,8 +1683,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);
@@ -1701,14 +1692,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);
--
1.6.3.3
--
Arvid Brodin
Enea Services Stockholm AB
--
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
