Re: [PATCH net-next v4 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 2 potential issue(s) to consider:
- [High] NULL Pointer Dereference in `lan966x_shutdown` due to uninitialized NAPI structure.
- [High] Unsafe Teardown: Network Queues Reactivate DMA Engine across host reboot.
--

commit 416539af976fe45bd91d93198a7588ced579e467
Author: Daniel Machon <daniel.machon@microchip.com>

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

When lan966x is used as a PCIe endpoint, the FDMA engine runs on the
card and survives a host reboot. Without a shutdown callback, channels
stay active and interrupt sources stay armed across the reset.

[ ... ]

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index b3701953b0905..271c023900dbe 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -1311,9 +1311,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);

Could this race with a concurrent transmission and re-activate the DMA
engine across reboot?

Because the network device queues aren't stopped before disabling the TX path
(e.g., via netif_device_detach() or netif_tx_disable()), the networking stack
remains active during shutdown.

If a packet is transmitted concurrently or after the callback runs,
ndo_start_xmit() (lan966x_fdma_xmit()) will execute, acquire tx_lock, and call
lan966x_fdma_tx_start(). Seeing that tx->activated is false, it will
unconditionally set it to true and call lan966x_fdma_tx_activate(tx).

This re-initializes and restarts the hardware DMA channel and explicitly
unmasks the FDMA_INTR_DB_ENA interrupts, which seems to defeat the purpose of
the shutdown callback.

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

Does this napi_disable() call risk a NULL pointer dereference during shutdown?

If the device tree contains no active child nodes under the ethernet-ports
node, or if port initialization fails, lan966x_probe() still returns success.

In this configuration, lan966x->fdma is set to true (since the IRQ was
acquired), but lan966x_fdma_netdev_init() is never invoked. This leaves the
lan966x->napi structure zero-initialized, with napi.dev remaining NULL.

When napi_disable() executes, it locks the associated netdev via
netdev_lock(n->dev), which will crash when n->dev is NULL.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-lan966x-pci-fdma-v4-0-14e0c89d8d63@microchip.com?part=8