cfg80211: fix deadlock
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>
This commit is contained in:
committed by
John W. Linville
parent
96909e9771
commit
0ff6ce7b36
@@ -586,9 +586,14 @@ void wiphy_unregister(struct wiphy *wiphy)
|
|||||||
* get to lock contention here if userspace issues a command
|
* get to lock contention here if userspace issues a command
|
||||||
* that identified the hardware by wiphy index.
|
* that identified the hardware by wiphy index.
|
||||||
*/
|
*/
|
||||||
mutex_lock(&rdev->mtx);
|
cfg80211_lock_rdev(rdev);
|
||||||
/* unlock again before freeing */
|
|
||||||
mutex_unlock(&rdev->mtx);
|
if (WARN_ON(rdev->scan_req)) {
|
||||||
|
rdev->scan_req->aborted = true;
|
||||||
|
___cfg80211_scan_done(rdev);
|
||||||
|
}
|
||||||
|
|
||||||
|
cfg80211_unlock_rdev(rdev);
|
||||||
|
|
||||||
cfg80211_debugfs_rdev_del(rdev);
|
cfg80211_debugfs_rdev_del(rdev);
|
||||||
|
|
||||||
@@ -605,7 +610,6 @@ void wiphy_unregister(struct wiphy *wiphy)
|
|||||||
|
|
||||||
flush_work(&rdev->scan_done_wk);
|
flush_work(&rdev->scan_done_wk);
|
||||||
cancel_work_sync(&rdev->conn_work);
|
cancel_work_sync(&rdev->conn_work);
|
||||||
kfree(rdev->scan_req);
|
|
||||||
flush_work(&rdev->event_work);
|
flush_work(&rdev->event_work);
|
||||||
}
|
}
|
||||||
EXPORT_SYMBOL(wiphy_unregister);
|
EXPORT_SYMBOL(wiphy_unregister);
|
||||||
@@ -653,6 +657,11 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
|
|||||||
|
|
||||||
switch (state) {
|
switch (state) {
|
||||||
case NETDEV_REGISTER:
|
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);
|
mutex_init(&wdev->mtx);
|
||||||
INIT_LIST_HEAD(&wdev->event_list);
|
INIT_LIST_HEAD(&wdev->event_list);
|
||||||
spin_lock_init(&wdev->event_lock);
|
spin_lock_init(&wdev->event_lock);
|
||||||
@@ -730,13 +739,11 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
|
|||||||
#endif
|
#endif
|
||||||
break;
|
break;
|
||||||
case NETDEV_UNREGISTER:
|
case NETDEV_UNREGISTER:
|
||||||
cfg80211_lock_rdev(rdev);
|
/*
|
||||||
|
* NB: cannot take rdev->mtx here because this may be
|
||||||
if (WARN_ON(rdev->scan_req && rdev->scan_req->dev == dev)) {
|
* called within code protected by it when interfaces
|
||||||
rdev->scan_req->aborted = true;
|
* are removed with nl80211.
|
||||||
___cfg80211_scan_done(rdev);
|
*/
|
||||||
}
|
|
||||||
|
|
||||||
mutex_lock(&rdev->devlist_mtx);
|
mutex_lock(&rdev->devlist_mtx);
|
||||||
/*
|
/*
|
||||||
* It is possible to get NETDEV_UNREGISTER
|
* It is possible to get NETDEV_UNREGISTER
|
||||||
@@ -755,7 +762,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
|
|||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
mutex_unlock(&rdev->devlist_mtx);
|
mutex_unlock(&rdev->devlist_mtx);
|
||||||
cfg80211_unlock_rdev(rdev);
|
|
||||||
break;
|
break;
|
||||||
case NETDEV_PRE_UP:
|
case NETDEV_PRE_UP:
|
||||||
if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
|
if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
|
||||||
|
Reference in New Issue
Block a user