linux-kernel - [PATCH v2] usb: oxu210hp-hcd: Fix double locking of oxu->mem

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20240921114914.8802-1-m.lobanov@rosalinux.ru>
Date: Sat, 21 Sep 2024 07:49:12 -0400
From: Mikhail Lobanov <m.lobanov@...alinux.ru>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Mikhail Lobanov <m.lobanov@...alinux.ru>,
	"Dr. David Alan Gilbert" <linux@...blig.org>,
	Rodolfo Giometti <giometti@...ux.it>,
	linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	lvc-project@...uxtesting.org
Subject: [PATCH v2] usb: oxu210hp-hcd: Fix double locking of oxu->mem_lock spinlock

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@...alinux.ru>
---
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@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