From: Alexander Duyck fbn->num_napi is the count of live napi vectors, each of which owns an IRQ. The PM path frees the IRQs and vectors without clearing the count. It must do this to avoid leaving too many IRQs active which could overwhelm CPU 0 on some systems and end up in the suspend failing. See b980c0634fe5 ("i40e: shutdown all IRQs and disable MSI-X when suspended") for more info. fbnic_pm_suspend() tears the datapath down via ndo_stop() and frees the IRQs, but leaves netif_running() true so resume knows to re-open. Resume rebuilds the datapath in __fbnic_pm_resume() and fbnic_reset_queues() overwrites num_napi and __fbnic_open() re-allocates the vectors. When the datapath is torn down but never rebuilt, num_napi is left pointing at freed vectors. Two cases that can trigger this are: - A PCIe error recovery that fails (fbnic_err_slot_reset() -> __fbnic_pm_resume() returns an error -> PCI_ERS_RESULT_DISCONNECT), so .resume never runs - An __fbnic_open() that fails partway on resume and unwinds, freeing the vectors after fbnic_reset_queues() has already set num_napi. The netdev is then running with num_napi > 0 but napi[] freed, and the eventual remove/unbind close re-enters fbnic_down() -> fbnic_dbg_down() and dereferences the freed vectors: BUG: kernel NULL pointer dereference, address: 0000000000000210 RIP: fbnic_dbg_down+0x28 Clear num_napi when the vectors in the suspend are torn down (a good resume re-establishes it before __fbnic_open()) and clear it on a resume open failure. A redundant ndo_stop() then walks an empty napi[]. The normal ndo_stop() down/up cycle is untouched and retains num_napi for the next ndo_open(). Fixes: bc6107771bb4 ("eth: fbnic: Allocate a netdevice and napi vectors with queues") Signed-off-by: Alexander Duyck --- drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c index 8b9bc9e8ea56..c6698e3002a1 100644 --- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c +++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c @@ -434,6 +434,7 @@ static int fbnic_pm_suspend(struct device *dev) { struct fbnic_dev *fbd = dev_get_drvdata(dev); struct net_device *netdev = fbd->netdev; + struct fbnic_net *fbn; if (fbnic_init_failure(fbd)) goto null_uc_addr; @@ -441,11 +442,16 @@ static int fbnic_pm_suspend(struct device *dev) rtnl_lock(); netdev_lock(netdev); + fbn = netdev_priv(netdev); + netif_device_detach(netdev); if (netif_running(netdev)) netdev->netdev_ops->ndo_stop(netdev); + /* The IRQs are about to be freed, so drop the napi vector count */ + fbn->num_napi = 0; + netdev_unlock(netdev); rtnl_unlock(); @@ -508,16 +514,20 @@ static int __fbnic_pm_resume(struct device *dev) if (fbnic_init_failure(fbd)) return 0; + rtnl_lock(); + netdev_lock(netdev); + fbn = netdev_priv(netdev); /* Reset the queues if needed */ fbnic_reset_queues(fbn, fbn->num_tx_queues, fbn->num_rx_queues); - rtnl_lock(); - netdev_lock(netdev); - - if (netif_running(netdev)) + if (netif_running(netdev)) { err = __fbnic_open(fbn); + /* On failure the vectors are freed, so drop the count */ + if (err) + fbn->num_napi = 0; + } netdev_unlock(netdev); rtnl_unlock();