Thread (24 messages) 24 messages, 5 authors, 8d ago

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

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Date: 2026-09-25 17:02:17
Also in: bpf

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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help