Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting

[Ming Lei <tom.leiming@xxxxxxxxx>]

On Fri, Jun 22, 2012 at 5:11 PM, Bjørn Mork <bjorn@xxxxxxx> wrote:
> usbnet_disconnect() will set intfdata to NULL before calling
> the minidriver unbind function.  The cdc_wdm subdriver cannot
> know that it is disconnecting until the qmi_wwan unbind
> function has called its disconnect function.  This means that
> we must be able to support the cdc_wdm subdriver operating
> normally while usbnet_disconnect() is running, and in
> particular that intfdata may be NULL.
>
> The only place this matters is in qmi_wwan_cdc_wdm_manage_power
> which is called from cdc_wdm.  Simply testing for NULL
> intfdata there is sufficient to allow it to continue working
> at all times.
>
> Fixes this Oops where a cdc-wdm device was closed while the
> USB device was disconnecting, causing wdm_release to call
> qmi_wwan_cdc_wdm_manage_power after intfdata was set to
> NULL by usbnet_disconnect:

In fact, it should be a general problem in usbnet, there are races
between usbnet_disconnect and .open/.close. Considered that
unregister_netdev, which will sync with .open/.close,  is called in
usbnet_disconnect,  the simplest fix is to move usb_set_intfdata(NULL)
after unregister_netdev.

> Reported-by: Marius Bjørnstad Kotsbak <marius.kotsbak@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.4
> Signed-off-by: Bjørn Mork <bjorn@xxxxxxx>
> ---
>  drivers/net/usb/qmi_wwan.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index 3767a12..b01960f 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -197,6 +197,10 @@ err:
>  static int qmi_wwan_cdc_wdm_manage_power(struct usb_interface *intf, int on)
>  {
>        struct usbnet *dev = usb_get_intfdata(intf);
> +
> +       /* can be called while disconnecting */
> +       if (!dev)
> +               return 0;
>        return qmi_wwan_manage_power(dev, on);
>  }

Considered that usb_set_intfdata(NULL) will be called after
executing .disconnect in unbind path, it can be removed simply.

So how about the below?

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index e92c057..2eb9e1e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1286,7 +1286,6 @@ void usbnet_disconnect (struct usb_interface *intf)
 	struct net_device	*net;

 	dev = usb_get_intfdata(intf);
-	usb_set_intfdata(intf, NULL);
 	if (!dev)
 		return;


Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html