Skip to content

Commit

Permalink
cfg80211: fix deadlock
Browse files Browse the repository at this point in the history
When removing an interface with nl80211, cfg80211 will
deadlock in the netdev notifier because we're already
holding rdev->mtx and try to acquire it again to verify
the scan has been done.

This bug was introduced by my patch
"cfg80211: check for and abort dangling scan requests".

To fix this, move the dangling scan request check into
wiphy_unregister(). This will not be able to catch all
cases right away, but if the scan problem happens with
a manual ifdown or so it will be possible to remedy it
by removing the module/device.

Additionally, add comments about the deadlock scenario.

Reported-by: Christian Lamparter <chunkeey@web.de>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Tested-by: Christian Lamparter <chunkeey@web.de>
Tested-by: Kalle Valo <kalle.valo@iki.fi>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
  • Loading branch information
Johannes Berg authored and John W. Linville committed Aug 20, 2009
1 parent 96909e9 commit 0ff6ce7
Showing 1 changed file with 18 additions and 12 deletions.
30 changes: 18 additions & 12 deletions net/wireless/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,14 @@ void wiphy_unregister(struct wiphy *wiphy)
* get to lock contention here if userspace issues a command
* that identified the hardware by wiphy index.
*/
mutex_lock(&rdev->mtx);
/* unlock again before freeing */
mutex_unlock(&rdev->mtx);
cfg80211_lock_rdev(rdev);

if (WARN_ON(rdev->scan_req)) {
rdev->scan_req->aborted = true;
___cfg80211_scan_done(rdev);
}

cfg80211_unlock_rdev(rdev);

cfg80211_debugfs_rdev_del(rdev);

Expand All @@ -605,7 +610,6 @@ void wiphy_unregister(struct wiphy *wiphy)

flush_work(&rdev->scan_done_wk);
cancel_work_sync(&rdev->conn_work);
kfree(rdev->scan_req);
flush_work(&rdev->event_work);
}
EXPORT_SYMBOL(wiphy_unregister);
Expand Down Expand Up @@ -653,6 +657,11 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,

switch (state) {
case NETDEV_REGISTER:
/*
* NB: cannot take rdev->mtx here because this may be
* called within code protected by it when interfaces
* are added with nl80211.
*/
mutex_init(&wdev->mtx);
INIT_LIST_HEAD(&wdev->event_list);
spin_lock_init(&wdev->event_lock);
Expand Down Expand Up @@ -730,13 +739,11 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
#endif
break;
case NETDEV_UNREGISTER:
cfg80211_lock_rdev(rdev);

if (WARN_ON(rdev->scan_req && rdev->scan_req->dev == dev)) {
rdev->scan_req->aborted = true;
___cfg80211_scan_done(rdev);
}

/*
* NB: cannot take rdev->mtx here because this may be
* called within code protected by it when interfaces
* are removed with nl80211.
*/
mutex_lock(&rdev->devlist_mtx);
/*
* It is possible to get NETDEV_UNREGISTER
Expand All @@ -755,7 +762,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
#endif
}
mutex_unlock(&rdev->devlist_mtx);
cfg80211_unlock_rdev(rdev);
break;
case NETDEV_PRE_UP:
if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
Expand Down

0 comments on commit 0ff6ce7

Please sign in to comment.