Re: [PATCH net-next v3 06/13] net: lan966x: add FDMA ops dispatch for PCIe support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Teardown asymmetry in `lan966x_remove()` frees FDMA memory before network devices are unregistered, creating a UAF window.
- [High] Premature network device registration exposes the interface before FDMA structures are allocated.
- [High] If `fdma_init()` fails during probe, the NAPI context is deleted while still enabled, violating networking subsystem rules.
--

commit 26b4dc8869b3940c369a8ef84d983f66881dd6b2
Author: Daniel Machon <daniel.machon@microchip.com>

net: lan966x: add FDMA ops dispatch for PCIe support

Introduce lan966x_fdma_ops to support different FDMA implementations
for platform and PCIe. Plumb fdma_init, fdma_deinit, fdma_xmit,
fdma_poll and fdma_resize through the ops table, and select the
implementation at probe time based on runtime PCI bus detection.

> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 47752d3fde0b1..9f69634ebb0ae 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c

[ ... ]

> @@ -1232,7 +1247,7 @@ static int lan966x_probe(struct platform_device *pdev)
>  	if (err)
>  		goto cleanup_fdb;
>  
> -	err = lan966x_fdma_init(lan966x);
> +	err = lan966x->ops->fdma_init(lan966x);

While reviewing this refactor, I noticed an existing issue regarding the
initialization order. Earlier in the probe sequence, the driver loops over
child nodes and calls lan966x_probe_port(), which executes register_netdev().

Once register_netdev() returns, the network device is fully visible to the
system and transmission can be initiated.

However, the FDMA memory arrays (like tx.dcbs_buf) are not allocated until
lan966x->ops->fdma_init(lan966x) is called here.

If a packet is sent between register_netdev() and fdma_init(), could
lan966x->ops->fdma_xmit() execute and dereference the unallocated tx->dcbs_buf
array?

Additionally, if lan966x->ops->fdma_init() fails, the error path jumps to
cleanup_ptp. This explicitly bypasses cleanup_fdma and skips
lan966x->ops->fdma_deinit().

Since NAPI is enabled early inside lan966x_probe_port() via
lan966x_fdma_netdev_init(), and fdma_deinit() (which contains the
napi_disable() call) is skipped, the NAPI context remains active.

The error path then proceeds to lan966x_cleanup_ports(), which ultimately
calls netif_napi_del().

Can deleting an active NAPI context without disabling it first lead to crashes
if the background polling task is concurrently scheduled?

>  	if (err)
>  		goto cleanup_ptp;

[ ... ]

> @@ -1273,7 +1288,7 @@ static void lan966x_remove(struct platform_device *pdev)
>  
>  	lan966x_taprio_deinit(lan966x);
>  	lan966x_vcap_deinit(lan966x);
> -	lan966x_fdma_deinit(lan966x);
> +	lan966x->ops->fdma_deinit(lan966x);

There also appears to be a similar pre-existing issue here during teardown.

lan966x->ops->fdma_deinit() cleanly frees the FDMA RX/TX structures, but the
network interfaces are still active because lan966x_cleanup_ports() (which
unregisters the netdevs) has not executed yet.

During this window, if userspace or the network stack triggers a transmission,
lan966x_port_xmit() will see port->lan966x->fdma as true and invoke
lan966x->ops->fdma_xmit().

Could this transmission routine dereference the recently freed tx->dcbs_buf
buffers, resulting in a use-after-free?

>  	lan966x_cleanup_ports(lan966x);
>  
>  	cancel_delayed_work_sync(&lan966x->stats_work);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260504-lan966x-pci-fdma-v3-0-a56f5740d870@microchip.com?part=6