linux-kernel - [PATCH] usb: host: oxu210hp-hcd: Fix potential deadlock on &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: <20230629121947.59315-1-dg573847474@gmail.com>
Date:   Thu, 29 Jun 2023 12:19:47 +0000
From:   Chengfeng Ye <dg573847474@...il.com>
To:     gregkh@...uxfoundation.org, gustavoars@...nel.org
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Chengfeng Ye <dg573847474@...il.com>
Subject: [PATCH] usb: host: oxu210hp-hcd: Fix potential deadlock on &oxu->mem_lock

As &udc->lock is acquired by watchdog timer oxu_watchdog() under
softirq context, other acquisition of the same lock under process
context should disable irq, otherwise deadlock could happen if the
irq preempt the execution while the lock is held in process context
on the same CPU.

The .urb_enqueue callback oxu_urb_enqueue() acquires the lock without
disabling irq inside the function.

Possible deadlock scenario

oxu_urb_enqueue()
    -> oxu_murb_alloc()
    -> spin_lock(&oxu->mem_lock)
        <timer interrupt>
        -> oxu_watchdog()
        -> ehci_work()
        -> scan_async()
        -> qh_completions()
        -> oxu_murb_free()
        -> spin_lock(&oxu->mem_lock)

This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock, which reported the above
warning when analyzing the linux kernel 6.4-rc7 release.

The tentative patch fixes the potential deadlock by spin_lock_irqsave().
x86_64 allyesconfig using GCC shows no new warning, the tool does not
report the warnining after the fix. No runtime testing was performed since
I don't have device with OXU210HP host controller.

Signed-off-by: Chengfeng Ye <dg573847474@...il.com>
---
 drivers/usb/host/oxu210hp-hcd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index f998d3f1a78a..944011e059e5 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -1120,8 +1120,9 @@ static struct oxu_murb *oxu_murb_alloc(struct oxu_hcd *oxu)
 {
 	int i;
 	struct oxu_murb *murb = NULL;
+	unsigned long flags;

-	spin_lock(&oxu->mem_lock);
+	spin_lock_irqsave(&oxu->mem_lock, flags);

 	for (i = 0; i < MURB_NUM; i++)
 		if (!oxu->murb_used[i])
@@ -1133,7 +1134,7 @@ static struct oxu_murb *oxu_murb_alloc(struct oxu_hcd *oxu)
 		oxu->murb_used[i] = 1;
 	}

-	spin_unlock(&oxu->mem_lock);
+	spin_unlock_irqrestore(&oxu->mem_lock, flags);

 	return murb;
 }
-- 
2.17.1