USB: dummy-hcd: Fix interrupt synchronization error

This fixes an error in synchronization in the dummy-hcd driver.  The
error has a somewhat involved history.  The synchronization mechanism
was introduced by commit 7dbd8f4cabd9 ("USB: dummy-hcd: Fix erroneous
synchronization change"), which added an emulated "interrupts enabled"
flag together with code emulating synchronize_irq() (it waits until
all current handler callbacks have returned).

But the emulated interrupt-disable occurred too late, after the driver
containing the handler callback routines had been told that it was
unbound and no more callbacks would occur.  Commit 4a5d797a9f9c ("usb:
gadget: dummy_hcd: fix gpf in gadget_setup") tried to fix this by
moving the synchronize_irq() emulation code from dummy_stop() to
dummy_pullup(), which runs before the unbind callback.

There still were races, though, because the emulated interrupt-disable
still occurred too late.  It couldn't be moved to dummy_pullup(),
because that routine can be called for reasons other than an impending
unbind.  Therefore commits 7dc0c55e9f30 ("USB: UDC core: Add
udc_async_callbacks gadget op") and 04145a03db9d ("USB: UDC: Implement
udc_async_callbacks in dummy-hcd") added an API allowing the UDC core
to tell dummy-hcd exactly when emulated interrupts and their callbacks
should be disabled.

That brings us to the current state of things, which is still wrong
because the emulated synchronize_irq() occurs before the emulated
interrupt-disable!  That's no good, beause it means that more emulated
interrupts can occur after the synchronize_irq() emulation has run,
leading to the possibility that a callback handler may be running when
the gadget driver is unbound.

To fix this, we have to move the synchronize_irq() emulation code yet
again, to the dummy_udc_async_callbacks() routine, which takes care of
enabling and disabling emulated interrupt requests.  The
synchronization will now run immediately after emulated interrupts are
disabled, which is where it belongs.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Fixes: 04145a03db9d ("USB: UDC: Implement udc_async_callbacks in dummy-hcd")
Cc: stable <stable@kernel.org>
Link: https://patch.msgid.link/c7bc93fe-4241-4d04-bd56-27c12ba35c97@rowland.harvard.edu
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
index 860ee7d3ef982fb9091df65a2e8c7fd0d5c1f5e6..e55701575857cadc75faffb660f7259aa0e3ffa2 100644 (file)
--- a/drivers/usb/gadget/udc/dummy_hcd.c
+++ b/drivers/usb/gadget/udc/dummy_hcd.c
@@ -913,21 +913,6 @@ static int dummy_pullup(struct usb_gadget *_gadget, int value)
        spin_lock_irqsave(&dum->lock, flags);
        dum->pullup = (value != 0);
        set_link_state(dum_hcd);
-       if (value == 0) {
-               /*
-                * Emulate synchronize_irq(): wait for callbacks to finish.
-                * This seems to be the best place to emulate the call to
-                * synchronize_irq() that's in usb_gadget_remove_driver().
-                * Doing it in dummy_udc_stop() would be too late since it
-                * is called after the unbind callback and unbind shouldn't
-                * be invoked until all the other callbacks are finished.
-                */
-               while (dum->callback_usage > 0) {
-                       spin_unlock_irqrestore(&dum->lock, flags);
-                       usleep_range(1000, 2000);
-                       spin_lock_irqsave(&dum->lock, flags);
-               }
-       }
        spin_unlock_irqrestore(&dum->lock, flags);
 
        usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd));
@@ -950,6 +935,20 @@ static void dummy_udc_async_callbacks(struct usb_gadget *_gadget, bool enable)
 
        spin_lock_irq(&dum->lock);
        dum->ints_enabled = enable;
+       if (!enable) {
+               /*
+                * Emulate synchronize_irq(): wait for callbacks to finish.
+                * This has to happen after emulated interrupts are disabled
+                * (dum->ints_enabled is clear) and before the unbind callback,
+                * just like the call to synchronize_irq() in
+                * gadget/udc/core:gadget_unbind_driver().
+                */
+               while (dum->callback_usage > 0) {
+                       spin_unlock_irq(&dum->lock);
+                       usleep_range(1000, 2000);
+                       spin_lock_irq(&dum->lock);
+               }
+       }
        spin_unlock_irq(&dum->lock);
 }