Re: [PATCH net] net: don't mix device locking in dev_close_many() calls

[Stanislav Fomichev <stfomichev@gmail.com>]

On 04/12, Jakub Kicinski wrote:
> Lockdep found the following dependency:
> 
>   &dev_instance_lock_key#3 -->
>      &rdev->wiphy.mtx -->
>         &net->xdp.lock -->
> 	   &xs->mutex -->
> 	      &dev_instance_lock_key#3
> 
> The first dependency is the problem. wiphy mutex should be outside
> the instance locks. The problem happens in notifiers (as always)
> for CLOSE. We only hold the instance lock for ops locked devices
> during CLOSE, and WiFi netdevs are not ops locked. Unfortunately,
> when we dev_close_many() during netns dismantle we may be holding
> the instance lock of _another_ netdev when issuing a CLOSE for
> a WiFi device.
> 
> Lockdep's "Possible unsafe locking scenario" only prints 3 locks
> and we have 4, plus I think we'd need 3 CPUs, like this:
> 
>        CPU0                 CPU1              CPU2
>        ----                 ----              ----
>   lock(&xs->mutex);
>                        lock(&dev_instance_lock_key#3);
>                                          lock(&rdev->wiphy.mtx);
>                                          lock(&net->xdp.lock);
>                                          lock(&xs->mutex);
>                        lock(&rdev->wiphy.mtx);
>   lock(&dev_instance_lock_key#3);
> 
> Tho, I don't think that's possible as CPU1 and CPU2 would
> be under rtnl_lock. Even if we have per-netns rtnl_lock and
> wiphy can span network namespaces - CPU0 and CPU1 must be
> in the same netns to see dev_instance_lock, so CPU0 can't
> be installing a socket as CPU1 is tearing the netns down.
> 
> Regardless, our expected lock ordering is that wiphy lock
> is taken before instance locks, so let's fix this.
> 
> Go over the ops locked and non-locked devices separately.
> Note that calling dev_close_many() on an empty list is perfectly
> fine. All processing (including RCU syncs) are conditional
> on the list not being empty, already.
> 
> Fixes: 7e4d784f5810 ("net: hold netdev instance lock during rtnetlink operations")
> Reported-by: syzbot+6f588c78bf765b62b450@syzkaller.appspotmail.com
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>