From: "Bjørn Mork" <bjorn@mork•no>
To: David Miller <davem@davemloft•net>
Cc: andreyknvl@gmail•com, torvalds@linux-foundation•org,
oneukum@suse•com, dvyukov@google•com, glider@google•com,
kcc@google•com, gregkh@linuxfoundation•org,
linux-usb@vger•kernel.org, netdev@vger•kernel.org
Subject: [PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind
Date: Mon, 7 Mar 2016 21:15:36 +0100 [thread overview]
Message-ID: <87k2le815w.fsf_-_@nemi.mork.no> (raw)
In-Reply-To: <20160307.145406.188091362181073486.davem@davemloft.net> (David Miller's message of "Mon, 07 Mar 2016 14:54:06 -0500 (EST)")
usbnet_link_change will call schedule_work and should be
avoided if bind is failing. Otherwise we will end up with
scheduled work referring to a netdev which has gone away.
Instead of making the call conditional, we can just defer
it to usbnet_probe, using the driver_info flag made for
this purpose.
Fixes: 8a34b0ae8778 ("usbnet: cdc_ncm: apply usbnet_link_change")
Reported-by: Andrey Konovalov <andreyknvl@gmail•com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation•org>
Signed-off-by: Bjørn Mork <bjorn@mork•no>
---
David Miller <davem@davemloft•net> writes:
> From: Andrey Konovalov <andreyknvl@gmail•com>
>
>> Could you also add:
>> Reported-by: Andrey Konovalov <andreyknvl@gmail•com>
>> ?
>
> Sorry it's already committed to my tree and I can't redo the commit message
> once that happens since my tree has static history.
Even with Oliver's generic fix we should still fix the inconsistency
in cdc_ncm, as pointed out by Linus.
This is a slightly different approach than the patch proposed by Linus.
When I started looking at this I couldn't figure out why we were doing
this differently in this driver from all the other usbnet drivers
disabling the link at probe time. So let's make it consistent. Then at
least we get consistent bugs :)
Bjørn
drivers/net/usb/cdc_ncm.c | 20 +++++---------------
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index be927964375b..86ba30ba35e8 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -988,8 +988,6 @@ EXPORT_SYMBOL_GPL(cdc_ncm_select_altsetting);
static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
{
- int ret;
-
/* MBIM backwards compatible function? */
if (cdc_ncm_select_altsetting(intf) != CDC_NCM_COMM_ALTSETTING_NCM)
return -ENODEV;
@@ -998,16 +996,7 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
* Additionally, generic NCM devices are assumed to accept arbitrarily
* placed NDP.
*/
- ret = cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
-
- /*
- * We should get an event when network connection is "connected" or
- * "disconnected". Set network connection in "disconnected" state
- * (carrier is OFF) during attach, so the IP network stack does not
- * start IPv6 negotiation and more.
- */
- usbnet_link_change(dev, 0, 0);
- return ret;
+ return cdc_ncm_bind_common(dev, intf, CDC_NCM_DATA_ALTSETTING_NCM, 0);
}
static void cdc_ncm_align_tail(struct sk_buff *skb, size_t modulus, size_t remainder, size_t max)
@@ -1590,7 +1579,8 @@ static void cdc_ncm_status(struct usbnet *dev, struct urb *urb)
static const struct driver_info cdc_ncm_info = {
.description = "CDC NCM",
- .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET,
+ .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
+ | FLAG_LINK_INTR,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
@@ -1603,7 +1593,7 @@ static const struct driver_info cdc_ncm_info = {
static const struct driver_info wwan_info = {
.description = "Mobile Broadband Network Device",
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
- | FLAG_WWAN,
+ | FLAG_LINK_INTR | FLAG_WWAN,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
@@ -1616,7 +1606,7 @@ static const struct driver_info wwan_info = {
static const struct driver_info wwan_noarp_info = {
.description = "Mobile Broadband Network Device (NO ARP)",
.flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
- | FLAG_WWAN | FLAG_NOARP,
+ | FLAG_LINK_INTR | FLAG_WWAN | FLAG_NOARP,
.bind = cdc_ncm_bind,
.unbind = cdc_ncm_unbind,
.manage_power = usbnet_manage_power,
--
2.1.4
next prev parent reply other threads:[~2016-03-07 20:48 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-04 21:26 Possible double-free in the usbnet driver Linus Torvalds
[not found] ` <CA+55aFxqwjs5gs6Fw2jmTteWM4hZTnr7Ls111ExNTieObLs82Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-04 22:26 ` Andrey Konovalov
[not found] ` <CA+fCnZfS51F7WZEM1YTSPDMWSzvBTJWGf5cRWv5LrNCSOf_-qA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-04 22:42 ` Oliver Neukum
[not found] ` <1457131342.8935.2.camel-l3A5Bk7waGM@public.gmane.org>
2016-03-04 23:00 ` Andrey Konovalov
2016-03-04 23:22 ` Andrey Konovalov
2016-03-04 22:43 ` Linus Torvalds
2016-03-04 23:00 ` Andrey Konovalov
2016-03-05 15:51 ` Oliver Neukum
[not found] ` <1457193090.8935.7.camel-IBi9RG/b67k@public.gmane.org>
2016-03-05 19:53 ` Bjørn Mork
[not found] ` <DBDB517D-E4A4-4422-AECE-52194FE2AED0-yOkvZcmFvRU@public.gmane.org>
2016-03-07 18:13 ` Linus Torvalds
[not found] ` <CA+55aFw43uKkUK-h=VMwXcb9NM+g2AdeB960kWb4YihmjQ8DRw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-07 19:11 ` David Miller
[not found] ` <20160307.141100.1511700720120062677.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2016-03-07 19:50 ` Andrey Konovalov
[not found] ` <CA+fCnZdurxGBsOrANb_m5BLK1BKzH3J_GmZ=dbH=ABThgFNGxg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-07 19:54 ` David Miller
2016-03-07 20:15 ` Bjørn Mork [this message]
[not found] ` <87k2le815w.fsf_-_-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-03-07 20:58 ` [PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind David Miller
2016-03-08 19:43 ` Linus Torvalds
[not found] ` <CA+55aFxt7zWW+-EkwCbAWCb9wkgVswYJNAz86bc_QRcv1pBHZw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-08 20:12 ` Oliver Neukum
2016-03-08 20:18 ` Bjørn Mork
[not found] ` <871t7k7mgo.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-03-08 20:20 ` Oliver Neukum
2016-03-08 20:37 ` Ben Hutchings
2016-03-07 21:39 ` Possible double-free in the usbnet driver Oliver Neukum
[not found] ` <1457386754.3404.15.camel-IBi9RG/b67k@public.gmane.org>
2016-03-08 11:42 ` Andrey Konovalov
[not found] ` <CA+55aFwxbs_hLG58Q_xSK2vpufjmwMk-xkqxTNh_5h-A8y4vbg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-03-07 9:08 ` Dmitry Vyukov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87k2le815w.fsf_-_@nemi.mork.no \
--to=bjorn@mork$(echo .)no \
--cc=andreyknvl@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=dvyukov@google$(echo .)com \
--cc=glider@google$(echo .)com \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=kcc@google$(echo .)com \
--cc=linux-usb@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=oneukum@suse$(echo .)com \
--cc=torvalds@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox