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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling netif_napi_del() on an active NAPI instance without prior disablement if fdma_init() fails.
- [High] unregister_netdev() is called on an unregistered network device if register_netdev() fails during probe.
--

commit 01a8933fefad605649fbdd137d3cd5c5de3e8ada
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 47752d3fde0b..9f69634ebb0a 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);
>  	if (err)
>  		goto cleanup_ptp;

This isn't a regression from this patch, but does this error path leave an
active napi instance?

In lan966x_probe(), the port initialization loop calls
lan966x_fdma_netdev_init(), which adds and enables napi via napi_enable().

Later in the probe sequence, lan966x->ops->fdma_init() is called. If it
fails, the error handling jumps to the cleanup_ptp label, bypassing
cleanup_fdma (which would have called fdma_deinit and napi_disable()).

The error path then proceeds to lan966x_cleanup_ports(), which calls
lan966x_fdma_netdev_deinit() to invoke netif_napi_del() on the
still-enabled napi instance. 

Could this corrupt the CPU's poll list and cause a use-after-free?

> @@ -1245,7 +1260,7 @@ static int lan966x_probe(struct platform_device *pdev)
>  	return 0;
>  
>  cleanup_fdma:
> -	lan966x_fdma_deinit(lan966x);
> +	lan966x->ops->fdma_deinit(lan966x);
>  
>  cleanup_ptp:
>  	lan966x_ptp_deinit(lan966x);

This also isn't a regression from this patch, but can this error path lead
to unregister_netdev() being called on an unregistered network device?

In lan966x_probe_port(), lan966x->ports[p] and port->dev are populated
before calling register_netdev(). If register_netdev() fails, the function
returns an error without clearing these pointers.

lan966x_probe() catches the error and jumps to cleanup_ports.

lan966x_cleanup_ports() iterates over all populated lan966x->ports[p] and
unconditionally calls unregister_netdev(port->dev).

Since the device failed to register, its reg_state is not NETREG_REGISTERED.
Will the networking core's unregister_netdevice() hit a BUG_ON() and crash
the system?

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