On Fri, Jul 31, 2020 at 04:57:58PM +0800, Macpaul Lin wrote:
> This patch fixes a possible issue when mtu3_gadget_stop()
> already assigned NULL to mtu->gadget_driver during mtu_gadget_disconnect().
>
> Signed-off-by: Macpaul Lin <macpaul....@mediatek.com>
> Cc: sta...@vger.kernel.org
> ---
> Changes for v2:
> - Check mtu_gadget_driver out of spin_lock might still not work.
> We use a temporary pointer to keep the callback function.
>
> drivers/usb/mtu3/mtu3_gadget.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
> index 68ea4395f871..40cb6626f496 100644
> --- a/drivers/usb/mtu3/mtu3_gadget.c
> +++ b/drivers/usb/mtu3/mtu3_gadget.c
> @@ -840,10 +840,17 @@ void mtu3_gadget_suspend(struct mtu3 *mtu)
> /* called when VBUS drops below session threshold, and in other cases */
> void mtu3_gadget_disconnect(struct mtu3 *mtu)
> {
> + struct usb_gadget_driver *driver;
> +
> dev_dbg(mtu->dev, "gadget DISCONNECT\n");
> if (mtu->gadget_driver && mtu->gadget_driver->disconnect) {
> + driver = mtu->gadget_driver;
> spin_unlock(&mtu->lock);
> - mtu->gadget_driver->disconnect(&mtu->g);
> + /*
> + * avoid kernel panic because mtu3_gadget_stop() assigned NULL
> + * to mtu->gadget_driver.
> + */
> + driver->disconnect(&mtu->g);
> spin_lock(&mtu->lock);
> }
This is not the right approach; it might race with the gadget driver
unregistering itself.
Instead, mtu3_gadget_stop() should call synchronize_irq() after
releasing the IRQ line. When synchronize_irq() returns, you'll know any
IRQ handlers have finished running, so you won't receive any more
disconnect notifications. Then it will be safe to acquire the spinlock
and set mtu->gadget_driver to NULL.
Alan Stern
