From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Jiri Slaby <jslaby@suse.cz>,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
"David S. Miller" <davem@davemloft.net>
Cc: stable@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 4.4 18/31] r8152: napi hangup fix after disconnect
Date: Wed, 12 Sep 2018 19:54:32 +0100 [thread overview]
Message-ID: <1536778472.3024.204.camel@codethink.co.uk> (raw)
In-Reply-To: <3b118091-d600-0be8-3204-c0f794ef6288@suse.cz>
On Sat, 2018-08-25 at 09:43 +0200, Jiri Slaby wrote:
> On 08/24/2018, 06:38 PM, Ben Hutchings wrote:
> > On Fri, 2018-07-20 at 14:13 +0200, Greg Kroah-Hartman wrote:
> > > 4.4-stable review patch. If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Jiri Slaby <jslaby@suse.cz>
> > >
> > > [ Upstream commit 0ee1f4734967af8321ecebaf9c74221ace34f2d5 ]
> >
> > [...]
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > > @@ -3139,7 +3139,8 @@ static int rtl8152_close(struct net_devi
> > > #ifdef CONFIG_PM_SLEEP
> > > unregister_pm_notifier(&tp->pm_notifier);
> > > #endif
> > > - napi_disable(&tp->napi);
> > > + if (!test_bit(RTL8152_UNPLUG, &tp->flags))
> > > + napi_disable(&tp->napi);
> > > clear_bit(WORK_ENABLE, &tp->flags);
> > > usb_kill_urb(tp->intr_urb);
> > > cancel_delayed_work_sync(&tp->schedule);
> >
> > This flag appears to be set only if the USB device is actually
> > disconnected. In case the driver is unbound for some other reason
> > (like the module is removed), the same problem will occur.
>
> Could you elaborate? I thought this would happen:
> module_exit -> usb_deregister -> usb_unbind_device -> rtl8152_disconnect
> -> unregister_netdev -> rtl8152_close
>
> Am I missing something?
What I mean is that if the USB device has not been *physically*
disconnected then its usb_device::state will not be
USB_STATE_NOTATTACHED. So rtl8152_disconnect() will not set the
RTL8152_UNPLUG flag and rtl8152_close() will still call napi_disable()
which will hang.
Some options to fix this:
- Add a separate flag which rtl8152_close() checks and
rtl8152_disconnect() always sets
- Call dev_close() before netif_napi_del()
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
WARNING: multiple messages have this Message-ID (diff)
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
To: Jiri Slaby <jslaby@suse.cz>,
linux-usb@vger.kernel.org, netdev@vger.kernel.org,
"David S. Miller" <davem@davemloft.net>
Cc: stable@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: [4.4,18/31] r8152: napi hangup fix after disconnect
Date: Wed, 12 Sep 2018 19:54:32 +0100 [thread overview]
Message-ID: <1536778472.3024.204.camel@codethink.co.uk> (raw)
On Sat, 2018-08-25 at 09:43 +0200, Jiri Slaby wrote:
> On 08/24/2018, 06:38 PM, Ben Hutchings wrote:
> > On Fri, 2018-07-20 at 14:13 +0200, Greg Kroah-Hartman wrote:
> > > 4.4-stable review patch. If anyone has any objections, please let me know.
> > >
> > > ------------------
> > >
> > > From: Jiri Slaby <jslaby@suse.cz>
> > >
> > > [ Upstream commit 0ee1f4734967af8321ecebaf9c74221ace34f2d5 ]
> >
> > [...]
> > > --- a/drivers/net/usb/r8152.c
> > > +++ b/drivers/net/usb/r8152.c
> > > @@ -3139,7 +3139,8 @@ static int rtl8152_close(struct net_devi
> > > #ifdef CONFIG_PM_SLEEP
> > > unregister_pm_notifier(&tp->pm_notifier);
> > > #endif
> > > - napi_disable(&tp->napi);
> > > + if (!test_bit(RTL8152_UNPLUG, &tp->flags))
> > > + napi_disable(&tp->napi);
> > > clear_bit(WORK_ENABLE, &tp->flags);
> > > usb_kill_urb(tp->intr_urb);
> > > cancel_delayed_work_sync(&tp->schedule);
> >
> > This flag appears to be set only if the USB device is actually
> > disconnected. In case the driver is unbound for some other reason
> > (like the module is removed), the same problem will occur.
>
> Could you elaborate? I thought this would happen:
> module_exit -> usb_deregister -> usb_unbind_device -> rtl8152_disconnect
> -> unregister_netdev -> rtl8152_close
>
> Am I missing something?
What I mean is that if the USB device has not been *physically*
disconnected then its usb_device::state will not be
USB_STATE_NOTATTACHED. So rtl8152_disconnect() will not set the
RTL8152_UNPLUG flag and rtl8152_close() will still call napi_disable()
which will hang.
Some options to fix this:
- Add a separate flag which rtl8152_close() checks and
rtl8152_disconnect() always sets
- Call dev_close() before netif_napi_del()
Ben.
next prev parent reply other threads:[~2018-09-12 18:54 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-20 12:13 [PATCH 4.4 00/31] 4.4.143-stable review Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 01/31] compiler, clang: suppress warning for unused static inline functions Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 02/31] compiler, clang: properly override inline for clang Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 03/31] compiler, clang: always inline when CONFIG_OPTIMIZE_INLINING is disabled Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 04/31] compiler-gcc.h: Add __attribute__((gnu_inline)) to all inline declarations Greg Kroah-Hartman
2018-07-20 12:13 ` Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 05/31] x86/asm: Add _ASM_ARG* constants for argument registers to <asm/asm.h> Greg Kroah-Hartman
2018-07-20 12:13 ` Greg Kroah-Hartman
2018-07-20 12:13 ` Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 06/31] Revert "sit: reload iphdr in ipip6_rcv" Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 07/31] ocfs2: subsystem.su_mutex is required while accessing the item->ci_parent Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 08/31] bcm63xx_enet: correct clock usage Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 09/31] bcm63xx_enet: do not write to random DMA channel on BCM6345 Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 10/31] crypto: crypto4xx - remove bad list_del Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 11/31] crypto: crypto4xx - fix crypto4xx_build_pdr, crypto4xx_build_sdr leak Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 12/31] atm: zatm: Fix potential Spectre v1 Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 13/31] net: dccp: avoid crash in ccid3_hc_rx_send_feedback() Greg Kroah-Hartman
2018-07-20 12:13 ` Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 14/31] net: dccp: switch rx_tstamp_last_feedback to monotonic clock Greg Kroah-Hartman
2018-07-20 12:13 ` Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 15/31] net/mlx5: Fix incorrect raw command length parsing Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 16/31] net: sungem: fix rx checksum support Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 17/31] qed: Limit msix vectors in kdump kernel to the minimum required count Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 18/31] r8152: napi hangup fix after disconnect Greg Kroah-Hartman
2018-07-20 12:13 ` [4.4,18/31] " Greg Kroah-Hartman
2018-08-24 16:38 ` [PATCH 4.4 18/31] " Ben Hutchings
2018-08-24 16:38 ` [4.4,18/31] " Ben Hutchings
2018-08-25 7:43 ` [PATCH 4.4 18/31] " Jiri Slaby
2018-08-25 7:43 ` [4.4,18/31] " Jiri Slaby
2018-09-12 18:54 ` Ben Hutchings [this message]
2018-09-12 18:54 ` Ben Hutchings
2018-07-20 12:13 ` [PATCH 4.4 19/31] tcp: fix Fast Open key endianness Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 21/31] vhost_net: validate sock before trying to put its fd Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 22/31] net_sched: blackhole: tell upper qdisc about dropped packets Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 23/31] net/mlx5: Fix command interface race in polling mode Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 24/31] net: cxgb3_main: fix potential Spectre v1 Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 25/31] rtlwifi: rtl8821ae: fix firmware is not ready to run Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 26/31] MIPS: Call dump_stack() from show_regs() Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 27/31] MIPS: Use async IPIs for arch_trigger_cpumask_backtrace() Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 28/31] netfilter: ebtables: reject non-bridge targets Greg Kroah-Hartman
2018-07-20 12:13 ` [PATCH 4.4 29/31] KEYS: DNS: fix parsing multiple options Greg Kroah-Hartman
2018-07-20 12:14 ` [PATCH 4.4 30/31] rds: avoid unenecessary cong_update in loop transport Greg Kroah-Hartman
2018-07-20 12:14 ` [PATCH 4.4 31/31] net/nfc: Avoid stalls when nfc_alloc_send_skb() returned NULL Greg Kroah-Hartman
2018-07-20 13:34 ` [PATCH 4.4 00/31] 4.4.143-stable review Nathan Chancellor
2018-07-21 7:38 ` Naresh Kamboju
2018-07-21 13:39 ` Guenter Roeck
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=1536778472.3024.204.camel@codethink.co.uk \
--to=ben.hutchings@codethink.co.uk \
--cc=davem@davemloft.net \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.cz \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stable@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.