Commit f0a6fd15 authored by Johannes Berg's avatar Johannes Berg
Browse files

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: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Fixes: ea6b2098 ("cfg80211: fix locking in netlink owner interface destruction")
Tested-by: default avatarToke Høiland-Jørgensen <toke@redhat.com>
Link: https://lore.kernel.org/r/20220201130951.22093-1-johannes@sipsolutions.net


Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent 5f06f6bf

net/wireless/core.c

+4 −13
Original line number Diff line number Diff line
@@ -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) {

			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)