linux-kernel - [PATCH] drivers/xillybus: fix deadlock upon cleanup

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>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240813040425.187418-1-snovitoll@gmail.com>
Date: Tue, 13 Aug 2024 09:04:25 +0500
From: Sabyrzhan Tasbolatov <snovitoll@...il.com>
To: eli.billauer@...il.com,
	arnd@...db.de,
	gregkh@...uxfoundation.org
Cc: linux-kernel@...r.kernel.org,
	Sabyrzhan Tasbolatov <snovitoll@...il.com>,
	syzbot+e528c9aad0fb5383ec83@...kaller.appspotmail.com
Subject: [PATCH] drivers/xillybus: fix deadlock upon cleanup_dev

syzbot found an issue [1] when cleanup_dev() is called twice,
causing deadlock. It is called in xillyusb_probe()
in the end of wakeup_all():

	INIT_WORK(&xdev->wakeup_workitem, wakeup_all);

then on "goto fail" path it's called again, where it's deadlocked in
destroy_workqueue(xdev->workq) due to first call's work_completion.

Also the deadlock prevents UAF as cleanup_dev frees struct xillyusb_dev.

It should be also the cause of previous syzbot report which was fixed in
commit 0d151a103775 ("Bluetooth: hci_core: cancel all works upon
hci_unregister_dev()").

[1]
WARNING: possible recursive locking detected
6.11.0-rc2-syzkaller-00004-gb446a2dae984 #0 Not tainted
--------------------------------------------
kworker/0:1H/58 is trying to acquire lock:
((wq_completion)xillyusb){+.+.}-{0:0},
((wq_completion)xillyusb){+.+.}-{0:0},

but task is already holding lock:
((wq_completion)xillyusb){+.+.}-{0:0},
((wq_completion)xillyusb){+.+.}-{0:0},

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock((wq_completion)xillyusb);
  lock((wq_completion)xillyusb);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

2 locks held by kworker/0:1H/58:
 #0: ((wq_completion)xillyusb){+.+.}-{0:0},
 #0: ((wq_completion)xillyusb){+.+.}-{0:0},
 #1: ((work_completion)(&xdev->wakeup_workitem)){+.+.}-{0:0},
 #1: ((work_completion)(&xdev->wakeup_workitem)){+.+.}-{0:0},

Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@...il.com>
Reported-by: syzbot+e528c9aad0fb5383ec83@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e528c9aad0fb5383ec83
Fixes: a53d1202aef1 ("char: xillybus: Add driver for XillyUSB (Xillybus variant for USB)")
---
 drivers/char/xillybus/xillyusb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 5a5afa14ca8c..099fe681aed5 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -2151,6 +2151,7 @@ static int xillyusb_probe(struct usb_interface *interface,
 	if (!xdev->workq) {
 		dev_err(&interface->dev, "Failed to allocate work queue\n");
 		rc = -ENOMEM;
+		kref_put(&xdev->kref, cleanup_dev);
 		goto fail;
 	}
 
@@ -2174,7 +2175,6 @@ static int xillyusb_probe(struct usb_interface *interface,
 
 fail:
 	usb_set_intfdata(interface, NULL);
-	kref_put(&xdev->kref, cleanup_dev);
 	return rc;
 }
 
-- 
2.34.1