Reset and remove can race, leaving NAPI registered and enabled:

modprobe idpf& sleep 1; ip link set eth0 up& rmmod idpf

  WARNING: net/core/dev.c:7699 at __netif_napi_del_locked+0x11a/0x130, CPU#30: 
rmmod/22393
  ...
  RIP: 0010:__netif_napi_del_locked+0x11a/0x130
  ...
  Call Trace:
  <TASK>
  free_netdev+0x110/0x2a0
  idpf_vport_dealloc+0x452/0x460 [idpf]
  idpf_deinit_task+0x51/0x70 [idpf]
  idpf_vc_core_deinit+0x32/0x170 [idpf]
  idpf_remove+0x40/0x200 [idpf]
  pci_device_remove+0x40/0xa0
  device_release_driver_internal+0x1a9/0x210
  driver_detach+0x4b/0x90
  bus_remove_driver+0x70/0x100
  pci_unregister_driver+0x2e/0xb0
  __do_sys_delete_module.constprop.0+0x190/0x2e0
  do_syscall_64+0xc8/0x6b0
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
  RIP: 0033:0x7fe628130beb

Make sure to call idpf_vport_stop() in idpf_stop(), irrespective of the
IDPF_REMOVE_IN_PROG state, to allow a reset racing with remove to tear
down NAPI in idpf_detach_and_close(), which runs under RTNL lock.

IDPF_VPORT_UP is modified under RTNL as idpf_vport_stop() and
idpf_up_complete() either take the lock themselves or are reached from
callbacks that do. Move the test inside the RTNL section so the test and
the teardown are atomic with respect to a racing close, and assert the
lock is held to keep the invariant documented and enforced.

Fixes: 2e281e1155fc ("idpf: detach and close netdevs while handling a reset")
Signed-off-by: Emil Tantilov <[email protected]>
---
 drivers/net/ethernet/intel/idpf/idpf_lib.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c 
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 2130bbef0c174..791ebff985072 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1005,12 +1005,14 @@ static void idpf_vport_stop(struct idpf_vport *vport, 
bool rtnl)
        struct idpf_queue_id_reg_info *chunks;
        u32 vport_id = vport->vport_id;
 
-       if (!test_bit(IDPF_VPORT_UP, np->state))
-               return;
-
        if (rtnl)
                rtnl_lock();
 
+       /* IDPF_VPORT_UP should only be changed under RTNL */
+       ASSERT_RTNL();
+       if (!test_bit(IDPF_VPORT_UP, np->state))
+               goto unlock;
+
        netif_carrier_off(vport->netdev);
        netif_tx_disable(vport->netdev);
 
@@ -1036,6 +1038,7 @@ static void idpf_vport_stop(struct idpf_vport *vport, 
bool rtnl)
        idpf_vport_intr_rel(rsrc);
        clear_bit(IDPF_VPORT_UP, np->state);
 
+unlock:
        if (rtnl)
                rtnl_unlock();
 }
@@ -1052,12 +1055,8 @@ static void idpf_vport_stop(struct idpf_vport *vport, 
bool rtnl)
  */
 static int idpf_stop(struct net_device *netdev)
 {
-       struct idpf_netdev_priv *np = netdev_priv(netdev);
        struct idpf_vport *vport;
 
-       if (test_bit(IDPF_REMOVE_IN_PROG, np->adapter->flags))
-               return 0;
-
        idpf_vport_ctrl_lock(netdev);
        vport = idpf_netdev_to_vport(netdev);
 
-- 
2.37.3

Reply via email to