[PATCH] usb host: Fix double locking of mutex 'oxu->mem_lock' in 'oxu_qh_alloc()'

Mikhail Lobanov posted 1 patch 2 months, 1 week ago
drivers/usb/host/oxu210hp-hcd.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
[PATCH] usb host: Fix double locking of mutex 'oxu->mem_lock' in 'oxu_qh_alloc()'
Posted by Mikhail Lobanov 2 months, 1 week ago
Initially, the function oxu_qh_alloc() acquired the mutex oxu->mem_lock,
and then called the function ehci_qtd_alloc(), which also attempted
to acquire the same mutex. This led to potential deadlocks.

Remove the locking from the function ehci_qtd_alloc(). Now, oxu_qh_alloc()
can call ehci_qtd_alloc() without causing double locking. In all other
cases where ehci_qtd_alloc() is called, acquire the mutex before the call,
maintaining mutex locking as in the previous implementation.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: b92a78e582b1 ("usb host: Oxford OXU210HP HCD driver.")
Signed-off-by: Mikhail Lobanov <m.lobanov@rosalinux.ru>
---
 drivers/usb/host/oxu210hp-hcd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index 3f871fe62b90..fa24cf89dadb 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -977,8 +977,6 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
 	int i;
 	struct ehci_qtd *qtd = NULL;
 
-	spin_lock(&oxu->mem_lock);
-
 	for (i = 0; i < QTD_NUM; i++)
 		if (!oxu->qtd_used[i])
 			break;
@@ -997,8 +995,6 @@ static struct ehci_qtd *ehci_qtd_alloc(struct oxu_hcd *oxu)
 		oxu->qtd_used[i] = 1;
 	}
 
-	spin_unlock(&oxu->mem_lock);
-
 	return qtd;
 }
 
@@ -1601,7 +1597,9 @@ static struct list_head *qh_urb_transaction(struct oxu_hcd *oxu,
 	/*
 	 * URBs map to sequences of QTDs: one logical transaction
 	 */
+	spin_lock(&oxu->mem_lock);
 	qtd = ehci_qtd_alloc(oxu);
+	spin_unlock(&oxu->mem_lock);
 	if (unlikely(!qtd))
 		return NULL;
 	list_add_tail(&qtd->qtd_list, head);
@@ -1630,7 +1628,9 @@ static struct list_head *qh_urb_transaction(struct oxu_hcd *oxu,
 		/* ... and always at least one more pid */
 		token ^= QTD_TOGGLE;
 		qtd_prev = qtd;
+		spin_lock(&oxu->mem_lock);
 		qtd = ehci_qtd_alloc(oxu);
+		spin_unlock(&oxu->mem_lock);
 		if (unlikely(!qtd))
 			goto cleanup;
 		qtd->urb = urb;
@@ -1686,7 +1686,9 @@ static struct list_head *qh_urb_transaction(struct oxu_hcd *oxu,
 			break;
 
 		qtd_prev = qtd;
+		spin_lock(&oxu->mem_lock);
 		qtd = ehci_qtd_alloc(oxu);
+		spin_unlock(&oxu->mem_lock);
 		if (unlikely(!qtd))
 			goto cleanup;
 		if (likely(len > 0)) {
@@ -1724,7 +1726,9 @@ static struct list_head *qh_urb_transaction(struct oxu_hcd *oxu,
 		}
 		if (one_more) {
 			qtd_prev = qtd;
+			spin_lock(&oxu->mem_lock);
 			qtd = ehci_qtd_alloc(oxu);
+			spin_unlock(&oxu->mem_lock);
 			if (unlikely(!qtd))
 				goto cleanup;
 			qtd->urb = urb;
-- 
2.43.0