cfg80211: fix race in netlink owner interface destruction

My previous fix here to fix the deadlock left a race where
the exact same deadlock (see the original commit referenced
below) can still happen if cfg80211_destroy_ifaces() already
runs while nl80211_netlink_notify() is still marking some
interfaces as nl_owner_dead.

The race happens because we have two loops here - first we
dev_close() all the netdevs, and then we destroy them. If we
also have two netdevs (first one need only be a wdev though)
then we can find one during the first iteration, close it,
and go to the second iteration -- but then find two, and try
to destroy also the one we didn't close yet.

Fix this by only iterating once.

Reported-by: Toke Høiland-Jørgensen <toke@redhat.com>
Fixes: ca6b57f4b107 ("cfg80211: fix locking in netlink owner interface destruction")
Tested-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20220201130951.22093-1-johannes@sipsolutions.net
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 3a54c8e6b6c61c87e40ed78459e99a4e69550aad..f08d4b3bb148bdb7f6177b784f49d717259cda2a 100644 (file)
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -5,7 +5,7 @@
  * Copyright 2006-2010         Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018-2021 Intel Corporation
+ * Copyright (C) 2018-2022 Intel Corporation
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -332,29 +332,20 @@ static void cfg80211_event_work(struct work_struct *work)
 void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev)
 {
        struct wireless_dev *wdev, *tmp;
-       bool found = false;
 
        ASSERT_RTNL();
 
-       list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
+       list_for_each_entry_safe(wdev, tmp, &rdev->wiphy.wdev_list, list) {
                if (wdev->nl_owner_dead) {
                        if (wdev->netdev)
                                dev_close(wdev->netdev);
-                       found = true;
-               }
-       }
-
-       if (!found)
-               return;
 
-       wiphy_lock(&rdev->wiphy);
-       list_for_each_entry_safe(wdev, tmp, &rdev->wiphy.wdev_list, list) {
-               if (wdev->nl_owner_dead) {
+                       wiphy_lock(&rdev->wiphy);
                        cfg80211_leave(rdev, wdev);
                        rdev_del_virtual_intf(rdev, wdev);
+                       wiphy_unlock(&rdev->wiphy);
                }
        }
-       wiphy_unlock(&rdev->wiphy);
 }
 
 static void cfg80211_destroy_iface_wk(struct work_struct *work)