Re: [PATCH net-next v5 08/13] net: lan966x: add shutdown callback to stop FDMA on reboot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Calling napi_disable() unconditionally during shutdown causes a NULL pointer dereference if the device tree contains no valid ports.
- [High] The lan966x_shutdown() callback disables hardware channels before stopping the network stack and NAPI, allowing concurrent traffic to re-activate the DMA channels.

Pre-existing issues:
- [Critical] The driver's `remove` method tears down the FDMA structures before unregistering the network devices, leading to a Use-After-Free upon concurrent transmission.
- [High] NAPI is deleted without being disabled in the probe error path if fdma_init fails.
--

commit 4567cfb4a8261113fe86f623b6d21c6d3a34dd6b
Author: Daniel Machon <daniel.machon@microchip.com>

net: lan966x: add shutdown callback to stop FDMA on reboot

This commit adds a shutdown callback to stop the FDMA engine when the host
reboots. It masks FDMA interrupts and disables the RX and TX channels.
This prevents the shared PCIe INTx from asserting before the driver
re-probes on the card.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c

[ ... ]

> static void lan966x_remove(struct platform_device *pdev)
> {
> 	struct lan966x *lan966x = platform_get_drvdata(pdev);
> 
> 	lan966x_taprio_deinit(lan966x);
> 	lan966x_vcap_deinit(lan966x);
> 	lan966x->ops->fdma_deinit(lan966x);
> 	lan966x_cleanup_ports(lan966x);

[Severity: Critical]
This is a pre-existing issue, but does the remove method tear down FDMA
structures before unregistering the network devices?

Because lan966x_cleanup_ports() has not yet unregistered the netdevs, they
remain active. Could a concurrent frame transmission call lan966x_fdma_xmit(),
access the freed DMA coherent memory, and result in a use-after-free?

> 
> 	cancel_delayed_work_sync(&lan966x->stats_work);
> 	destroy_workqueue(lan966x->stats_queue);

[ ... ]

> @@ -1313,9 +1313,27 @@ static void lan966x_remove(struct platform_device *pdev)
>  	debugfs_remove_recursive(lan966x->debugfs_root);
>  }
>  
> +static void lan966x_shutdown(struct platform_device *pdev)
> +{
> +	struct lan966x *lan966x = platform_get_drvdata(pdev);
> +
> +	if (!lan966x->fdma)
> +		return;
> +
> +	lan966x_fdma_rx_disable(&lan966x->rx);
> +	lan966x_fdma_tx_disable(&lan966x->tx);

[Severity: High]
Should the network stack and NAPI be disabled before turning off the
hardware channels?

If the networking stack invokes ndo_start_xmit concurrently, could the driver
observe the channel is deactivated and write to FDMA_CH_ACTIVATE to turn the
DMA engine back on? This might defeat the purpose of the shutdown callback
and risk DMA memory corruption.

> +
> +	napi_synchronize(&lan966x->napi);
> +	napi_disable(&lan966x->napi);

[Severity: High]
What happens here if the device tree contains no valid ports?

If the ports list is empty, lan966x_fdma_netdev_init() is never called, leaving
lan966x->napi zero-initialized (meaning napi->dev is NULL). Does calling
napi_disable() here unconditionally attempt to acquire netdev_lock(n->dev) and
cause a NULL pointer dereference?

> +
> +	lan_wr(0, lan966x, FDMA_INTR_ENA);
> +	lan_wr(0, lan966x, FDMA_INTR_DB_ENA);
> +}

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in
lan966x_probe() during error handling.

If lan966x->ops->fdma_init() fails after the global NAPI instance is
initialized and enabled by lan966x_port_init(), the error path jumps to
cleanup_ptp and bypasses cleanup_fdma.

This leads to lan966x_cleanup_ports() removing the NAPI instance via
netif_napi_del() without first calling napi_disable(). Does deleting an
enabled NAPI instance trigger a WARN_ON and potentially lead to a
use-after-free if a poll happens concurrently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-lan966x-pci-fdma-v5-0-ca56197ae05b@microchip.com?part=8