Re: [PATCH net-next v6 05/11] net: wangxun: move reusable PCI driver ops functions into libwx
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-03-31 00:55:37
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- net: wangxun: move reusable PCI driver ops functions into libwx This commit adds a function pointer for close_suspend and adjusts do_reset. These changes allow PCI driver operations such as shutdown to be called directly from the common libwx library.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c index 746623fa59b40..c65240ca3c283 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -3342,5 +3342,90 @@ void wx_service_timer(struct timer_list *t) } EXPORT_SYMBOL(wx_service_timer); +static void wx_dev_shutdown(struct pci_dev *pdev, bool *enable_wake) +{ + struct wx *wx = pci_get_drvdata(pdev); + struct net_device *netdev; + u32 wufc = wx->wol; + + netdev = wx->netdev; + rtnl_lock(); + netif_device_detach(netdev); + + if (netif_running(netdev)) + wx->close_suspend(wx); + + wx_clear_interrupt_scheme(wx); + rtnl_unlock(); + + if (wufc) { + wx_set_rx_mode(netdev); + wx_configure_rx(wx);
Does this sequence result in a use-after-free of the DMA rings? When the interface is running, wx->close_suspend() is called, which frees all receive and transmit descriptor rings via dma_free_coherent() in wx_free_resources(). If Wake-on-LAN is enabled (wufc != 0), the code then calls wx_configure_rx(). This programs the freed DMA physical addresses from ring->dma (which is not cleared upon free) into the MACs Receive Descriptor Base Address registers. If the MAC fetches from these freed pages, could it interpret garbage data as buffer addresses and DMA incoming network packets into random physical memory?
+ wr32(wx, WX_PSR_WKUP_CTL, wufc);
+ } else {
+ wr32(wx, WX_PSR_WKUP_CTL, 0);
+ }
+ pci_wake_from_d3(pdev, !!wufc);
+ *enable_wake = !!wufc;
+ wx_control_hw(wx, false);
+
+ pci_disable_device(pdev);
+}[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c index 50a13dcf4b3c8..baffcadb22690 100644 --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -604,11 +570,11 @@ static void ngbe_reinit_locked(struct wx *wx) clear_bit(WX_STATE_RESETTING, wx->state); } -void ngbe_do_reset(struct net_device *netdev) +void ngbe_do_reset(struct net_device *netdev, bool reinit) { struct wx *wx = netdev_priv(netdev); - if (netif_running(netdev)) + if (netif_running(netdev) && reinit) ngbe_reinit_locked(wx); }
Is it intentional that ngbe_do_reset() does nothing when reinit is false? Before this patch, ngbe_resume() explicitly called ngbe_reset_hw(). Now that it uses the shared wx_resume(), it calls wx->do_reset(netdev, false). Since ngbe_do_reset() lacks an else branch for when reinit is false (unlike txgbe_do_reset()), the hardware reset is skipped entirely during resume. Will this leave the MAC/PHY uninitialized and break the interface after a suspend/resume cycle?