Initially, the function oxu_qh_alloc() acquired the spinlock oxu->mem_lock,
and then called the function ehci_qtd_alloc(), which also attempted
to acquire the same spinlock. This would lead to a deadlock.
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 spinlock before the
call, maintaining spinlock 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@xxxxxxxxxxxx>
---
v1->v2: remove bogus mutex mentioning from commit description and update
the commit message to use spinlock.
link to v1: https://lore.kernel.org/lkml/20240916174326.118495-1-m.lobanov@xxxxxxxxxxxx/
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
