netdev - Re: iAVF circular lock dependency due to netdev

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: <20250205172325.5f7c8969@kernel.org>
Date: Wed, 5 Feb 2025 17:23:25 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: iAVF circular lock dependency due to netdev_lock

On Wed, 5 Feb 2025 15:20:07 -0800 Jacob Keller wrote:
> This happens because the driver takes netdev_lock prior to acquiring its
> own adapter->crit_lock, but then it calls register_netdevice under the
> crit_lock. Since commit 5fda3f35349b ("net: make netdev_lock() protect
> netdev->reg_state"), the register_netdevice() function now acquires
> netdev_lock as part of its flow.
> 
> I can fix this by refactoring iavf to only take netdev_lock after
> acquiring its own crit_lock.. but that smells funny. It seems like a
> future change could require to take netdev_lock before calling into the
> driver routines somehow, making that ordering problematic.
> 
> I'm not sure how else to fix this... I briefly considered just removing
> crit_lock and relying solely on netdev_lock for synchronization, but
> that doesn't work because of the register_netdevice() taking the lock.
> 
> I guess I could do some funky stuff with unlocking but that seems ugly
> as well...
> 
> I'm not sure what we should do to fix this.

Not sure either, the locking in this driver is quite odd. Do you know
why it's registering the netdev from a workqueue, and what the entry
points to the driver are?

Normally before the netdev is registered it can't get called, so all 
the locking is moot. But IDK if we need to protect from some FW
interactions, maybe?