Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c64391f2 authored by Arvid Brodin's avatar Arvid Brodin Committed by Greg Kroah-Hartman
Browse files

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: default avatarArvid Brodin <arvid.brodin@enea.com>
Tested-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent e08f6a27

drivers/usb/host/isp1760-hcd.c

+12 −18
Original line number Original line Diff line number Diff line
@@ -937,7 +937,6 @@ void schedule_ptds(struct usb_hcd *hcd) 
	struct isp1760_hcd *priv;

	struct isp1760_hcd *priv;

	struct isp1760_qh *qh, *qh_next;

	struct isp1760_qh *qh, *qh_next;

	struct list_head *ep_queue;

	struct list_head *ep_queue;

	struct usb_host_endpoint *ep;

	LIST_HEAD(urb_list);

	LIST_HEAD(urb_list);

	struct urb_listitem *urb_listitem, *urb_listitem_next;

	struct urb_listitem *urb_listitem, *urb_listitem_next;

	int i;

	int i;
@@ -955,17 +954,9 @@ void schedule_ptds(struct usb_hcd *hcd) 
	for (i = 0; i < QH_END; i++) {

	for (i = 0; i < QH_END; i++) {

		ep_queue = &priv->qh_list[i];

		ep_queue = &priv->qh_list[i];

		list_for_each_entry_safe(qh, qh_next, ep_queue, qh_list) {

		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);

			collect_qtds(hcd, qh, &urb_list);

			if (list_empty(&qh->qtd_list)) {

			if (list_empty(&qh->qtd_list))

				list_del(&qh->qh_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);

	struct isp1760_hcd *priv = hcd_to_priv(hcd);

	unsigned long spinflags;

	unsigned long spinflags;

	struct isp1760_qh *qh;

	struct isp1760_qh *qh, *qh_iter;

	struct isp1760_qtd *qtd;

	int i;





	spin_lock_irqsave(&priv->lock, spinflags);

	spin_lock_irqsave(&priv->lock, spinflags);
@@ -1717,14 +1708,17 @@ static void isp1760_endpoint_disable(struct usb_hcd *hcd, 
	if (!qh)

	if (!qh)

		goto out;

		goto out;





	list_for_each_entry(qtd, &qh->qtd_list, qtd_list)

	WARN_ON(!list_empty(&qh->qtd_list));

		if (qtd->status != QTD_RETIRE) {

			dequeue_urb_from_qtd(hcd, qh, qtd);

			qtd->urb->status = -ECONNRESET;

		}





	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;

	ep->hcpriv = NULL;

	/* Cannot free qh here since it will be parsed by schedule_ptds() */





	schedule_ptds(hcd);

	schedule_ptds(hcd);