diff mbox series

char: xillybus: Don't destroy workqueue from work item running on it

Message ID 20240801121126.60183-1-eli.billauer@gmail.com (mailing list archive)
State Accepted
Commit ccbde4b128ef9c73d14d0d7817d68ef795f6d131
Headers show
Series char: xillybus: Don't destroy workqueue from work item running on it | expand

Commit Message

Eli Billauer Aug. 1, 2024, 12:11 p.m. UTC
Triggered by a kref decrement, destroy_workqueue() may be called from
within a work item for destroying its own workqueue. This illegal
situation is averted by adding a module-global workqueue for exclusive
use of the offending work item. Other work items continue to be queued
on per-device workqueues to ensure performance.

Reported-by: syzbot+91dbdfecdd3287734d8e@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/lkml/0000000000000ab25a061e1dfe9f@google.com/
Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
---
 drivers/char/xillybus/xillyusb.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Greg Kroah-Hartman Aug. 3, 2024, 6:56 a.m. UTC | #1
On Thu, Aug 01, 2024 at 03:11:26PM +0300, Eli Billauer wrote:
> Triggered by a kref decrement, destroy_workqueue() may be called from
> within a work item for destroying its own workqueue. This illegal
> situation is averted by adding a module-global workqueue for exclusive
> use of the offending work item. Other work items continue to be queued
> on per-device workqueues to ensure performance.
> 
> Reported-by: syzbot+91dbdfecdd3287734d8e@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/lkml/0000000000000ab25a061e1dfe9f@google.com/
> Signed-off-by: Eli Billauer <eli.billauer@gmail.com>
> ---
>  drivers/char/xillybus/xillyusb.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)

What commit id does this fix?  Should it also go to stable kernels?

thanks,

greg k-h
Eli Billauer Aug. 3, 2024, 3:40 p.m. UTC | #2
On 03/08/2024 9:56, Greg KH wrote:
> What commit id does this fix?  Should it also go to stable kernels?

The bug was there from the very beginning. The fixed commit ID is 
a53d1202aef1 ("char: xillybus: Add driver for XillyUSB (Xillybus variant 
for USB)").

So yes, the fix is relevant for stable kernels as well.

Apologies for omitting the Fixes tag. I wasn't aware of it until now.

Thanks,
    Eli
diff mbox series

Patch

diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c
index 5a5afa14ca8c..33ca0f4af390 100644
--- a/drivers/char/xillybus/xillyusb.c
+++ b/drivers/char/xillybus/xillyusb.c
@@ -50,6 +50,7 @@  MODULE_LICENSE("GPL v2");
 static const char xillyname[] = "xillyusb";
 
 static unsigned int fifo_buf_order;
+static struct workqueue_struct *wakeup_wq;
 
 #define USB_VENDOR_ID_XILINX		0x03fd
 #define USB_VENDOR_ID_ALTERA		0x09fb
@@ -569,10 +570,6 @@  static void cleanup_dev(struct kref *kref)
  * errors if executed. The mechanism relies on that xdev->error is assigned
  * a non-zero value by report_io_error() prior to queueing wakeup_all(),
  * which prevents bulk_in_work() from calling process_bulk_in().
- *
- * The fact that wakeup_all() and bulk_in_work() are queued on the same
- * workqueue makes their concurrent execution very unlikely, however the
- * kernel's API doesn't seem to ensure this strictly.
  */
 
 static void wakeup_all(struct work_struct *work)
@@ -627,7 +624,7 @@  static void report_io_error(struct xillyusb_dev *xdev,
 
 	if (do_once) {
 		kref_get(&xdev->kref); /* xdev is used by work item */
-		queue_work(xdev->workq, &xdev->wakeup_workitem);
+		queue_work(wakeup_wq, &xdev->wakeup_workitem);
 	}
 }
 
@@ -2258,6 +2255,10 @@  static int __init xillyusb_init(void)
 {
 	int rc = 0;
 
+	wakeup_wq = alloc_workqueue(xillyname, 0, 0);
+	if (!wakeup_wq)
+		return -ENOMEM;
+
 	if (LOG2_INITIAL_FIFO_BUF_SIZE > PAGE_SHIFT)
 		fifo_buf_order = LOG2_INITIAL_FIFO_BUF_SIZE - PAGE_SHIFT;
 	else
@@ -2265,11 +2266,16 @@  static int __init xillyusb_init(void)
 
 	rc = usb_register(&xillyusb_driver);
 
+	if (rc)
+		destroy_workqueue(wakeup_wq);
+
 	return rc;
 }
 
 static void __exit xillyusb_exit(void)
 {
+	destroy_workqueue(wakeup_wq);
+
 	usb_deregister(&xillyusb_driver);
 }