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

On Fri, Jun 22, 2012 at 5:11 PM, Bjørn Mork [off-list ref] wrote:

Considered there isn't any protection between usbnet_disconnet and
wdm_open/wdm_close, you patch still doesn't work on the race, see
below:

   +       if (!dev)
   +               return 0;

            -> usbnet_disconnect() triggered and completed, so


            return qmi_wwan_manage_power(dev, on);




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

Sorry for mistaken triggered sending, :-(

On Sat, Jun 23, 2012 at 11:32 AM, Ming Lei [off-list ref] wrote:

On Fri, Jun 22, 2012 at 5:11 PM, Bjørn Mork [off-list ref] wrote:

quoted

Reported-by: Marius Bjørnstad Kotsbak <redacted>
Cc: <redacted> # v3.4
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 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 there isn't any protection between usbnet_disconnet and
wdm_open/wdm_close, you patch still doesn't work on the race, see
below:

   +       if (!dev)
   +               return 0;

            -> usbnet_disconnect() is triggered and completed here, so
            dev may point to a freed usbnet instance.

             return qmi_wwan_manage_power(dev, on);

One fix I can think of is to export wdm_mutex or its lock/unlock
functions, and introduce the function of qmi_wwan_disconnect, which
will call usbnet_disconnect with holding wdm_mutex.


Thanks,
-- 
Ming Lei