Re: [PATCH net 4/8] i40e: avoid deadlock when calling unregister_netdev()

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

On Tue, Sep 22, 2026 at 01:31:10AM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 1 (1) · Medium: 0 (2) · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [High] Incomplete fix: the RTNL self-deadlock is only removed from the
>   i40e_vsi_reinit_setup() arm of i40e_setup_pf_switch().
> 
> Pre-existing issues:
> - [High] Dangling main-VSI references after a failed VSI reinit.
> - [Medium] NULL pointer dereference when the reinit unwind re-enters
>   ndo_stop after q_vectors/rings were released.
> - [Medium] Memory leak in the asymmetric unwind of
>   i40e_vsi_reinit_setup() as it exists at this commit: the two early…
> 
> --
> 
> i40e: avoid deadlock when calling unregister_netdev()
> 
> This adds a lock_acquired parameter to i40e_vsi_reinit_setup() so the
> err_netdev unwind uses unregister_netdevice() when RTNL is already held by
> the rebuild path, and unregister_netdev() otherwise. The caller in
> i40e_setup_pf_switch() forwards its own lock_acquired flag.
> 
> > diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> > index 65aa50330aac..5ea8731ece08 100644
> > --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> > +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> 
> [ ... ]

> [Severity: High]
> Should the other arm of this if/else get the same treatment? Both branches
> are entered with identical lock state:
> 
> i40e_rebuild() {
> 	...
> 	if (!lock_acquired)
> 		rtnl_lock();
> 	ret = i40e_setup_pf_switch(pf, reinit, true);
> 	...
> }
> 
> but i40e_vsi_setup() still takes RTNL itself, both on success:
> 
> 	ret = register_netdev(vsi->netdev);
> 	if (ret)
> 		goto err_dl_port;
> 	vsi->netdev_registered = true;
> 
> and on its unwind, which is the exact pattern this patch just replaced:
> 
> err_msix:
> 	if (vsi->netdev_registered) {
> 		vsi->netdev_registered = false;
> 		unregister_netdev(vsi->netdev);
> 		free_netdev(vsi->netdev);
> 		vsi->netdev = NULL;
> 	}
> 
> Can the !main_vsi branch be reached with RTNL held? The err_netdev path
> patched above ends in i40e_vsi_clear(), which sets pf->vsi[vsi->idx] = NULL
> and kfree()s the VSI without resetting pf->lan_vsi, so
> i40e_pf_get_main_vsi() returns NULL on the next reset. The following
> rebuild would then take:
> 
> 		if (!main_vsi)
> 			main_vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN,
> 						  uplink_seid, 0);
> 
> with rtnl_mutex already held, and register_netdev() -> rtnl_lock() would
> self-deadlock on the non-recursive mutex, which is the failure mode this
> commit describes fixing.

seems i40e_vsi_setup needs the same teaching regarding lock being held

> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918212458.550425-1-anthony.l.nguyen%40intel.com