From: Oliver Neukum <oneukum@suse.com>
To: "Jan Klötzke" <jan@kloetzke.net>, "David Miller" <davem@davemloft.net>
Cc: Jan.Kloetzke@preh.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org
Subject: usbnet: fix kernel crash after disconnect
Date: Mon, 29 Apr 2019 20:48:08 +0200 [thread overview]
Message-ID: <1556563688.20085.31.camel@suse.com> (raw)
On Fr, 2019-04-19 at 09:17 +0200, Jan Klötzke wrote:
> Hi David,
>
> On Thu, Apr 18, 2019 at 04:35:44PM -0700, David Miller wrote:
> > From: Kloetzke Jan <Jan.Kloetzke@preh.de>
> > Date: Thu, 18 Apr 2019 08:02:59 +0000
> >
> > > I think this assumption is not correct. As far as I understand the
> > > networking code it is still possible that the ndo_start_xmit callback
> > > is called while ndo_stop is running and even after ndo_stop has
> > > returned. You can only be sure after unregister_netdev() has returned.
> > > Maybe some networking folks can comment on that.
> >
> > The kernel loops over the devices being unregistered, and first it clears
> > the __LINK_STATE_START on all of them, then it invokes ->ndo_stop() on
> > all of them.
> >
> > __LINK_STATE_START controls what netif_running() returns.
> >
> > All calls to ->ndo_start_xmit() are guarded by netif_running() checks.
> >
> > So when ndo_stop is invoked you should get no more ndo_start_xmit
> > invocations on that device. Otherwise how could you shut down DMA
> > resources and turn off the TX engine properly?
>
> But you could still race with another CPU that is past the
> netif_running() check, can you? So the driver has to make sure that it
> gracefully handles concurrent ->ndo_start_xmit() and ->ndo_stop() calls.
Looking at dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
this indeed seems possible. But the documentation says that it is not.
Dave?
> Or are there any locks/barriers involved that make sure all
> ->ndo_start_xmit() calls have returned before invoking ->ndo_stop()?
Jan,
could you make versio of your patch that gives a WARNing if this race
triggers?
Regards
Oliver
WARNING: multiple messages have this Message-ID (diff)
From: Oliver Neukum <oneukum@suse.com>
To: "Jan Klötzke" <jan@kloetzke.net>, "David Miller" <davem@davemloft.net>
Cc: Jan.Kloetzke@preh.de, linux-usb@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] usbnet: fix kernel crash after disconnect
Date: Mon, 29 Apr 2019 20:48:08 +0200 [thread overview]
Message-ID: <1556563688.20085.31.camel@suse.com> (raw)
Message-ID: <20190429184808.Xzc0MgubjJEDEb2i2a5grJ8utG4VywsAfOlzTSYodHY@z> (raw)
In-Reply-To: <20190419071752.GG1084@tuxedo>
On Fr, 2019-04-19 at 09:17 +0200, Jan Klötzke wrote:
> Hi David,
>
> On Thu, Apr 18, 2019 at 04:35:44PM -0700, David Miller wrote:
> > From: Kloetzke Jan <Jan.Kloetzke@preh.de>
> > Date: Thu, 18 Apr 2019 08:02:59 +0000
> >
> > > I think this assumption is not correct. As far as I understand the
> > > networking code it is still possible that the ndo_start_xmit callback
> > > is called while ndo_stop is running and even after ndo_stop has
> > > returned. You can only be sure after unregister_netdev() has returned.
> > > Maybe some networking folks can comment on that.
> >
> > The kernel loops over the devices being unregistered, and first it clears
> > the __LINK_STATE_START on all of them, then it invokes ->ndo_stop() on
> > all of them.
> >
> > __LINK_STATE_START controls what netif_running() returns.
> >
> > All calls to ->ndo_start_xmit() are guarded by netif_running() checks.
> >
> > So when ndo_stop is invoked you should get no more ndo_start_xmit
> > invocations on that device. Otherwise how could you shut down DMA
> > resources and turn off the TX engine properly?
>
> But you could still race with another CPU that is past the
> netif_running() check, can you? So the driver has to make sure that it
> gracefully handles concurrent ->ndo_start_xmit() and ->ndo_stop() calls.
Looking at dev_direct_xmit(struct sk_buff *skb, u16 queue_id)
this indeed seems possible. But the documentation says that it is not.
Dave?
> Or are there any locks/barriers involved that make sure all
> ->ndo_start_xmit() calls have returned before invoking ->ndo_stop()?
Jan,
could you make versio of your patch that gives a WARNing if this race
triggers?
Regards
Oliver
next reply other threads:[~2019-04-29 18:48 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-29 18:48 Oliver Neukum [this message]
2019-04-29 18:48 ` [PATCH] usbnet: fix kernel crash after disconnect Oliver Neukum
2019-04-30 14:15 ` [v2] " Kloetzke Jan
2019-04-30 14:15 ` [PATCH v2] " Kloetzke Jan
2019-05-05 7:45 ` [v2] " David Miller
2019-05-05 7:45 ` [PATCH v2] " David Miller
2019-05-06 8:17 ` Oliver Neukum
2019-05-16 7:10 ` Kloetzke Jan
2019-05-21 0:09 ` David Miller
2019-05-21 9:48 ` Oliver Neukum
2019-05-21 10:12 ` Kloetzke Jan
2019-05-21 11:42 ` Oliver Neukum
2019-05-21 13:18 ` [PATCH v3] " Kloetzke Jan
2019-05-21 20:47 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2019-04-30 14:08 Kloetzke Jan
2019-04-30 14:08 ` [PATCH] " Kloetzke Jan
2019-04-19 7:17 Jan Klötzke
2019-04-19 7:17 ` [PATCH] " Jan Klötzke
2019-04-18 23:35 David Miller
2019-04-18 23:35 ` [PATCH] " David Miller
2019-04-18 8:02 Kloetzke Jan
2019-04-18 8:02 ` [PATCH] " Kloetzke Jan
2019-04-18 6:37 Oliver Neukum
2019-04-18 6:37 ` [PATCH] " Oliver Neukum
2019-04-17 9:19 Kloetzke Jan
2019-04-17 9:19 ` [PATCH] " Kloetzke Jan
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=1556563688.20085.31.camel@suse.com \
--to=oneukum@suse.com \
--cc=Jan.Kloetzke@preh.de \
--cc=davem@davemloft.net \
--cc=jan@kloetzke.net \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.