Re: [BUG] usb: gadgetfs: KASAN null-ptr-deref and intermittent UAF in ep_aio_cancel()

[Alan Stern]

On Mon, Sep 07, 2026 at 10:00:00PM +0900, Minseo Kim wrote:
> Hi Alan,
> 
> > Is it okay to add your Tested-by: tag?
> 
> Yes, please add:
> 
> Tested-by: Minseo Kim <neck3922@xxxxxxxxx>

Will do.

> Thank you for the time and care you have put into this fix. I have learned
> a great deal while working through this issue with you.

You're welcome.

In your earlier testing, didn't you find that with the unpatched driver, 
the driver module's usage count remained elevated as long as the 
ep_user_copy_worker() was pending on the workqueue?  And therefore it 
was impossible to unload the module while a work item was still queued 
or running?

This patch introduces the possibility of that happening.  Therefore I 
would like to have a second patch, which flushes the workqueue and 
prevents the kernel from trying to run code in an unloaded module, ready 
to submit along with the first one.

My version of this second patch (meant to apply on top of the first 
patch) is below.  It is closely based on the version you wrote, the main 
difference being that it uses a single workqueue for both work routines.  
Could you test it and verify that it prevents the problem you observed 
with only the first patch installed?

Thank,

Alan Stern



Index: usb-devel/drivers/usb/gadget/legacy/inode.c
===================================================================
--- usb-devel.orig/drivers/usb/gadget/legacy/inode.c
+++ usb-devel/drivers/usb/gadget/legacy/inode.c
@@ -237,6 +237,7 @@ static const char *CHIP;
 static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
 
 static DEFINE_SPINLOCK(aio_lock);	/* Protect aio cancellation info */
+static struct workqueue_struct *gadgetfs_wq;
 
 /*----------------------------------------------------------------------*/
 
@@ -514,9 +515,7 @@ static int ep_aio_cancel(struct kiocb *i
 		spin_unlock_irqrestore(&aio_lock, flags);
 		return 0;	/* ep_aio() will call us again if needed */
 	}
-
 	priv->cancel_state = AIO_UNLINKING;
-	spin_unlock_irqrestore(&aio_lock, flags);
 
 	/*
 	 * We are called with the aio core holding iocb's context lock.
@@ -527,7 +526,9 @@ static int ep_aio_cancel(struct kiocb *i
 	 * For this reason, do the dequeue operation in a work routine.
 	 */
 	INIT_WORK(&priv->unlink_work, ep_unlink_worker);
-	schedule_work(&priv->unlink_work);
+	queue_work(gadgetfs_wq, &priv->unlink_work);
+
+	spin_unlock_irqrestore(&aio_lock, flags);
 	return 0;
 }
 
@@ -597,7 +598,7 @@ static void ep_aio_complete(struct usb_e
 
 	if (new_req_state == AIO_COMPLETED) {
 		INIT_WORK(&priv->copy_work, ep_user_copy_worker);
-		schedule_work(&priv->copy_work);
+		queue_work(gadgetfs_wq, &priv->copy_work);
 	}
 	if (cancel_state != AIO_UNLINKING) {
 		usb_ep_free_request(ep, req);
@@ -1773,6 +1774,8 @@ gadgetfs_unbind (struct usb_gadget *gadg
 	spin_unlock_irq (&dev->lock);
 
 	destroy_ep_files (dev);
+	flush_workqueue(gadgetfs_wq);
+
 	gadget->ep0->driver_data = NULL;
 	set_gadget_data (gadget, NULL);
 
@@ -2245,10 +2248,17 @@ static int __init gadgetfs_init (void)
 {
 	int status;
 
+	gadgetfs_wq = alloc_workqueue("gadgetfs", WQ_PERCPU, 0);
+	if (!gadgetfs_wq)
+		return -ENOMEM;
+
 	status = register_filesystem (&gadgetfs_type);
-	if (status == 0)
-		pr_info ("%s: %s, version " DRIVER_VERSION "\n",
-			shortname, driver_desc);
+	if (status) {
+		destroy_workqueue(gadgetfs_wq);
+		return status;
+	}
+
+	pr_info("%s: %s, version " DRIVER_VERSION "\n", shortname, driver_desc);
 	return status;
 }
 module_init (gadgetfs_init);
@@ -2257,6 +2267,7 @@ static void __exit gadgetfs_cleanup (voi
 {
 	pr_debug ("unregister %s\n", shortname);
 	unregister_filesystem (&gadgetfs_type);
+	destroy_workqueue(gadgetfs_wq);
 }
 module_exit (gadgetfs_cleanup);