| 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 |
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
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 --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); }
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(-)