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>

authored by

Alan Stern and committed by

Greg Kroah-Hartman 2 months ago 2ca9e46f 616a63ff

+14 -15

1 changed file

expand all

drivers

usb

gadget

udc

dummy_hcd.c

+14 -15

drivers/usb/gadget/udc/dummy_hcd.c

··· 913 913 spin_lock_irqsave(&dum->lock, flags); 914 914 dum->pullup = (value != 0); 915 915 set_link_state(dum_hcd); 916 - if (value == 0) { 917 - /* 918 - * Emulate synchronize_irq(): wait for callbacks to finish. 919 - * This seems to be the best place to emulate the call to 920 - * synchronize_irq() that's in usb_gadget_remove_driver(). 921 - * Doing it in dummy_udc_stop() would be too late since it 922 - * is called after the unbind callback and unbind shouldn't 923 - * be invoked until all the other callbacks are finished. 924 - */ 925 - while (dum->callback_usage > 0) { 926 - spin_unlock_irqrestore(&dum->lock, flags); 927 - usleep_range(1000, 2000); 928 - spin_lock_irqsave(&dum->lock, flags); 929 - } 930 - } 931 916 spin_unlock_irqrestore(&dum->lock, flags); 932 917 933 918 usb_hcd_poll_rh_status(dummy_hcd_to_hcd(dum_hcd)); ··· 935 950 936 951 spin_lock_irq(&dum->lock); 937 952 dum->ints_enabled = enable; 953 + if (!enable) { 954 + /* 955 + * Emulate synchronize_irq(): wait for callbacks to finish. 956 + * This has to happen after emulated interrupts are disabled 957 + * (dum->ints_enabled is clear) and before the unbind callback, 958 + * just like the call to synchronize_irq() in 959 + * gadget/udc/core:gadget_unbind_driver(). 960 + */ 961 + while (dum->callback_usage > 0) { 962 + spin_unlock_irq(&dum->lock); 963 + usleep_range(1000, 2000); 964 + spin_lock_irq(&dum->lock); 965 + } 966 + } 938 967 spin_unlock_irq(&dum->lock); 939 968 } 940 969

Configure Feed

Configure Feed