Re: [PATCH] net: lan966x: fix double fwnode_handle_put() and missing mdb deinit in probe error path

[Daniel Machon]

Hi Selby,

> In lan966x_probe(), fwnode_handle_put(ports) is called after the
> fwnode_for_each_available_child_node(ports, portnp) loop finishes.
> However, if lan966x_fdb_init(), lan966x_ptp_init(), lan966x_fdma_init(),
> or lan966x_vcap_init() subsequently fails, execution jumps to or falls
> through to cleanup_ports, calling fwnode_handle_put(ports) a second time
> and skipping lan966x_mdb_deinit().
> 
> Add cleanup_mdb and cleanup_switch labels so post-loop initialization
> failures deinitialize MDB and clean up ports without calling
> fwnode_handle_put() on ports again.
> 
> Signed-off-by: Selby Mashiki <mashikiselby@xxxxxxxxx>
> ---
>  drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 1179a6e12..7abfd97ad 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -1228,7 +1228,7 @@ static int lan966x_probe(struct platform_device *pdev)
>  	lan966x_mdb_init(lan966x);
>  	err = lan966x_fdb_init(lan966x);
>  	if (err)
> -		goto cleanup_ports;
> +		goto cleanup_mdb;
>  
>  	err = lan966x_ptp_init(lan966x);
>  	if (err)
> @@ -1255,10 +1255,15 @@ static int lan966x_probe(struct platform_device *pdev)
>  cleanup_fdb:
>  	lan966x_fdb_deinit(lan966x);
>  
> +cleanup_mdb:
> +	lan966x_mdb_deinit(lan966x);
> +	goto cleanup_switch;
> +
>  cleanup_ports:
>  	fwnode_handle_put(ports);
>  	fwnode_handle_put(portnp);
>  
> +cleanup_switch:
>  	lan966x_cleanup_ports(lan966x);
>  
>  	cancel_delayed_work_sync(&lan966x->stats_work);
> -- 
> 2.56.0

This might fix the mentioned deinit issues listed in the commit description,
however it is still racy with the netdevs, which are unregistered at a later
point.

As I mentioned in my lan966x-pci series [1], I will redo the whole error/deinit
path in a series for net-next, which will fix it for good, including the issues
in this patch.

I can't reject this with pw-bot, since I am currently not listed as a maintainer
for lan966x. Horatiu is pre-occupied with PHY stuff, so he probably wont reply
either. I will add a patch that adds me to maintainers once net-next opens, so
patches are routed properly to me.

[1] https://lore.kernel.org/netdev/20261002090248.xwlcm3zclt6imaou@DEN-DL-M70577/

/Daniel