netdev - Re: [PATCH iwl-net v2 2/2] ixgbe: destroy aci.lock later within ixgbe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9561e65d-0942-49d2-8dfc-790b3b24e9e6@intel.com>
Date: Mon, 8 Sep 2025 14:25:22 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
CC: <anthony.l.nguyen@...el.com>, <netdev@...r.kernel.org>,
	<intel-wired-lan@...ts.osuosl.org>, Koichiro Den <den@...inux.co.jp>
Subject: Re: [PATCH iwl-net v2 2/2] ixgbe: destroy aci.lock later within
 ixgbe_remove path

On 9/8/25 13:26, Jedrzej Jagielski wrote:
> There's another issue with aci.lock and previous patch uncovers it.
> aci.lock is being destroyed during removing ixgbe while some of the
> ixgbe closing routines are still ongoing. These routines use Admin
> Command Interface which require taking aci.lock which has been already
> destroyed what leads to call trace.
> 
> [  +0.000004] DEBUG_LOCKS_WARN_ON(lock->magic != lock)
> [  +0.000007] WARNING: CPU: 12 PID: 10277 at kernel/locking/mutex.c:155 mutex_lock+0x5f/0x70
> [  +0.000002] Call Trace:
> [  +0.000003]  <TASK>
> [  +0.000006]  ixgbe_aci_send_cmd+0xc8/0x220 [ixgbe]
> [  +0.000049]  ? try_to_wake_up+0x29d/0x5d0
> [  +0.000009]  ixgbe_disable_rx_e610+0xc4/0x110 [ixgbe]
> [  +0.000032]  ixgbe_disable_rx+0x3d/0x200 [ixgbe]
> [  +0.000027]  ixgbe_down+0x102/0x3b0 [ixgbe]
> [  +0.000031]  ixgbe_close_suspend+0x28/0x90 [ixgbe]
> [  +0.000028]  ixgbe_close+0xfb/0x100 [ixgbe]
> [  +0.000025]  __dev_close_many+0xae/0x220
> [  +0.000005]  dev_close_many+0xc2/0x1a0
> [  +0.000004]  ? kernfs_should_drain_open_files+0x2a/0x40
> [  +0.000005]  unregister_netdevice_many_notify+0x204/0xb00
> [  +0.000006]  ? __kernfs_remove.part.0+0x109/0x210
> [  +0.000006]  ? kobj_kset_leave+0x4b/0x70
> [  +0.000008]  unregister_netdevice_queue+0xf6/0x130
> [  +0.000006]  unregister_netdev+0x1c/0x40
> [  +0.000005]  ixgbe_remove+0x216/0x290 [ixgbe]
> [  +0.000021]  pci_device_remove+0x42/0xb0
> [  +0.000007]  device_release_driver_internal+0x19c/0x200
> [  +0.000008]  driver_detach+0x48/0x90
> [  +0.000003]  bus_remove_driver+0x6d/0xf0
> [  +0.000006]  pci_unregister_driver+0x2e/0xb0
> [  +0.000005]  ixgbe_exit_module+0x1c/0xc80 [ixgbe]
> 
> Same as for the previous commit, the issue has been highlighted by the
> commit 337369f8ce9e ("locking/mutex: Add MUTEX_WARN_ON() into fast path").
> 
> Move destroying aci.lock to the end of ixgbe_remove(), as this
> simply fixes the issue.
> 
> Fixes: 4600cdf9f5ac ("ixgbe: Enable link management in E610 device")
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>

thank you for fixing that!

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

note:
I didn't checked how it goes with git, but on "human level" it does not
conflict with moving devlink_free() to the end too:
https://patchwork.ozlabs.org/project/intel-wired-lan/list/?series=471779


> ---
>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 18cae81dc794..4aa4ca603584 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -11891,10 +11891,8 @@ static void ixgbe_remove(struct pci_dev *pdev)
>   	set_bit(__IXGBE_REMOVING, &adapter->state);
>   	cancel_work_sync(&adapter->service_task);
>   
> -	if (adapter->hw.mac.type == ixgbe_mac_e610) {
> +	if (adapter->hw.mac.type == ixgbe_mac_e610)
>   		ixgbe_disable_link_status_events(adapter);
> -		mutex_destroy(&adapter->hw.aci.lock);
> -	}
>   
>   	if (adapter->mii_bus)
>   		mdiobus_unregister(adapter->mii_bus);
> @@ -11954,6 +11952,9 @@ static void ixgbe_remove(struct pci_dev *pdev)
>   	disable_dev = !test_and_set_bit(__IXGBE_DISABLED, &adapter->state);
>   	free_netdev(netdev);
>   
> +	if (adapter->hw.mac.type == ixgbe_mac_e610)
> +		mutex_destroy(&adapter->hw.aci.lock);
> +
>   	if (disable_dev)
>   		pci_disable_device(pdev);
>   }