From patchwork Fri Jun 22 12:45:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ming Lei X-Patchwork-Id: 166591 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 5F366B6FA3 for ; Fri, 22 Jun 2012 22:45:18 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761604Ab2FVMpP (ORCPT ); Fri, 22 Jun 2012 08:45:15 -0400 Received: from mail-gh0-f174.google.com ([209.85.160.174]:62964 "EHLO mail-gh0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755631Ab2FVMpN convert rfc822-to-8bit (ORCPT ); Fri, 22 Jun 2012 08:45:13 -0400 Received: by ghrr11 with SMTP id r11so1460369ghr.19 for ; Fri, 22 Jun 2012 05:45:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=lTzw7lHGt+uYj/2Ic7/bBGYRhM8eJEo+PMHmKyRecFY=; b=K9fAU/laKYp9DTJKCpzIlwu1GDRs0GjIwhXwIlvf15aPAcoBwaeqjvbIfKmoupUR+q GrbbvasRw7QWq+w2O2qPX8OTQBIpFsFGn9aByU4pRMxZNGsm8DDeVInbs358jWGG3xvh v5eiDfdhVRwOWxlj2Rew8JkliewRJ2YTdmfx9RBeDUMB325XVa/NRagRRAnlGonXNBSY tSu7ZX6xzykO+vIt8K6fc5zaZGzivDt1PW1QXwt93iq/yyvNNtFhE3v/PZABzKum31gU YW8+O5AAY9GsqE4ug1uqgOqR79OQEgCNHJtQ5gqLGi/BVjjKBjqs2QLW+tmANtROrHT2 4iFg== MIME-Version: 1.0 Received: by 10.60.28.101 with SMTP id a5mr1770264oeh.69.1340369112560; Fri, 22 Jun 2012 05:45:12 -0700 (PDT) Received: by 10.182.197.10 with HTTP; Fri, 22 Jun 2012 05:45:12 -0700 (PDT) In-Reply-To: <1340356279-3124-1-git-send-email-bjorn@mork.no> References: <1340356279-3124-1-git-send-email-bjorn@mork.no> Date: Fri, 22 Jun 2012 20:45:12 +0800 Message-ID: Subject: Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting From: Ming Lei To: =?ISO-8859-1?Q?Bj=F8rn_Mork?= Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, =?ISO-8859-1?Q?Marius_Bj=F8rnstad_Kotsbak?= Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Jun 22, 2012 at 5:11 PM, Bjørn Mork 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 > Cc: # v3.4 > Signed-off-by: Bjørn Mork > --- >  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? Thanks, 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;