[2/2] cfg80211: pmsr: fix MAC address setting

Commit Message

Luca Coelho Feb. 6, 2019, 5:59 a.m. UTC

From: Johannes Berg <[e-mail]>

When we destroy the interface we already hold the wdev->mtx
while calling cfg80211_pmsr_wdev_down(), which assumes this
isn't true and flushes the worker that takes the lock, thus
leading to a deadlock.

Fix this by refactoring the worker and calling its code in
cfg80211_pmsr_wdev_down() directly.

We still need to flush the work later to make sure it's not
still running and will crash, but it will not do anything.

Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API")
Signed-off-by: Johannes Berg <[e-mail]>
---
 net/wireless/core.c |  2 ++
 net/wireless/pmsr.c | 22 +++++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

Comments

Luca Coelho Feb. 6, 2019, 6:01 a.m. UTC | #1

On Wed, 2019-02-06 at 07:59 +0200, Luca Coelho wrote:
> From: Johannes Berg <[e-mail]>
> 
> When we destroy the interface we already hold the wdev->mtx
> while calling cfg80211_pmsr_wdev_down(), which assumes this
> isn't true and flushes the worker that takes the lock, thus
> leading to a deadlock.
> 
> Fix this by refactoring the worker and calling its code in
> cfg80211_pmsr_wdev_down() directly.
> 
> We still need to flush the work later to make sure it's not
> still running and will crash, but it will not do anything.
> 
> Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM
> initiator API")
> Signed-off-by: Johannes Berg <[e-mail]>
> ---

Oops, this came out with the wrong subject, please ignore it.  I'll
resend with the correct one.

--
Luca.

Patch

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 623dfe5e211c..b36ad8efb5e5 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1068,6 +1068,8 @@  static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
 
 	ASSERT_RTNL();
 
+	flush_work(&wdev->pmsr_free_wk);
+
 	nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
 
 	list_del_rcu(&wdev->list);
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index f2e388e329fd..78c3f5633692 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -529,14 +529,14 @@  void cfg80211_pmsr_report(struct wireless_dev *wdev,
 }
 EXPORT_SYMBOL_GPL(cfg80211_pmsr_report);
 
-void cfg80211_pmsr_free_wk(struct work_struct *work)
+static void cfg80211_pmsr_process_abort(struct wireless_dev *wdev)
 {
-	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
-						 pmsr_free_wk);
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_pmsr_request *req, *tmp;
 	LIST_HEAD(free_list);
 
+	lockdep_assert_held(&wdev->mtx);
+
 	spin_lock_bh(&wdev->pmsr_lock);
 	list_for_each_entry_safe(req, tmp, &wdev->pmsr_list, list) {
 		if (req->nl_portid)
@@ -546,14 +546,22 @@  void cfg80211_pmsr_free_wk(struct work_struct *work)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	list_for_each_entry_safe(req, tmp, &free_list, list) {
-		wdev_lock(wdev);
 		rdev_abort_pmsr(rdev, wdev, req);
-		wdev_unlock(wdev);
 
 		kfree(req);
 	}
 }
 
+void cfg80211_pmsr_free_wk(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 pmsr_free_wk);
+
+	wdev_lock(wdev);
+	cfg80211_pmsr_process_abort(wdev);
+	wdev_unlock(wdev);
+}
+
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 {
 	struct cfg80211_pmsr_request *req;
@@ -567,8 +575,8 @@  void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	if (found)
-		schedule_work(&wdev->pmsr_free_wk);
-	flush_work(&wdev->pmsr_free_wk);
+		cfg80211_pmsr_process_abort(wdev);
+
 	WARN_ON(!list_empty(&wdev->pmsr_list));
 }