diff options
author | Eliad Peller <eliad@wizery.com> | 2013-12-05 18:30:17 +0200 |
---|---|---|
committer | Johannes Berg <johannes.berg@intel.com> | 2013-12-05 19:06:47 +0100 |
commit | 4a58e7c38443154fce1b47910e1a9184f65c5d72 (patch) | |
tree | 312b02efaf0de0522cca0eebfac2b5b861430d68 /net | |
parent | a2b70e833e189a4aefb2d3b668e3d7046dcc45c2 (diff) |
cfg80211: don't "leak" uncompleted scans
___cfg80211_scan_done() can be called in some cases
(e.g. on NETDEV_DOWN) before the low level driver
notified scan completion (which is indicated by
passing leak=true).
Clearing rdev->scan_req in this case is buggy, as
scan_done_wk might have already being queued/running
(and can't be flushed as it takes rtnl()).
If a new scan will be requested at this stage, the
scan_done_wk will try freeing it (instead of the
previous scan), and this will later result in
a use after free.
Simply remove the "leak" option, and replace it with
a standard WARN_ON.
An example backtrace after such crash:
Unable to handle kernel paging request at virtual address fffffee5
pgd = c0004000
[fffffee5] *pgd=9fdf6821, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] SMP ARM
PC is at cfg80211_scan_done+0x28/0xc4 [cfg80211]
LR is at __ieee80211_scan_completed+0xe4/0x2dc [mac80211]
[<bf0077b0>] (cfg80211_scan_done+0x28/0xc4 [cfg80211])
[<bf0973d4>] (__ieee80211_scan_completed+0xe4/0x2dc [mac80211])
[<bf0982cc>] (ieee80211_scan_work+0x94/0x4f0 [mac80211])
[<c005fd10>] (process_one_work+0x1b0/0x4a8)
[<c0060404>] (worker_thread+0x138/0x37c)
[<c0066d70>] (kthread+0xa4/0xb0)
Signed-off-by: Eliad Peller <eliad@wizery.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Diffstat (limited to 'net')
-rw-r--r-- | net/wireless/core.c | 20 | ||||
-rw-r--r-- | net/wireless/core.h | 2 | ||||
-rw-r--r-- | net/wireless/scan.c | 16 |
3 files changed, 8 insertions, 30 deletions
diff --git a/net/wireless/core.c b/net/wireless/core.c index c0443558b7bf..1a92c6a0731f 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -203,17 +203,8 @@ void cfg80211_stop_p2p_device(struct cfg80211_registered_device *rdev, rdev->opencount--; - if (rdev->scan_req && rdev->scan_req->wdev == wdev) { - /* - * If the scan request wasn't notified as done, set it - * to aborted and leak it after a warning. The driver - * should have notified us that it ended at the latest - * during rdev_stop_p2p_device(). - */ - if (WARN_ON(!rdev->scan_req->notified)) - rdev->scan_req->aborted = true; - ___cfg80211_scan_done(rdev, !rdev->scan_req->notified); - } + WARN_ON(rdev->scan_req && rdev->scan_req->wdev == wdev && + !rdev->scan_req->notified); } static int cfg80211_rfkill_set_block(void *data, bool blocked) @@ -859,11 +850,8 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb, break; case NETDEV_DOWN: cfg80211_update_iface_num(rdev, wdev->iftype, -1); - if (rdev->scan_req && rdev->scan_req->wdev == wdev) { - if (WARN_ON(!rdev->scan_req->notified)) - rdev->scan_req->aborted = true; - ___cfg80211_scan_done(rdev, true); - } + WARN_ON(rdev->scan_req && rdev->scan_req->wdev == wdev && + !rdev->scan_req->notified); if (WARN_ON(rdev->sched_scan_req && rdev->sched_scan_req->dev == wdev->netdev)) { diff --git a/net/wireless/core.h b/net/wireless/core.h index ac77644cf894..453c6ed880f1 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -361,7 +361,7 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev, struct key_params *params, int key_idx, bool pairwise, const u8 *mac_addr); void __cfg80211_scan_done(struct work_struct *wk); -void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak); +void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev); void __cfg80211_sched_scan_results(struct work_struct *wk); int __cfg80211_stop_sched_scan(struct cfg80211_registered_device *rdev, bool driver_initiated); diff --git a/net/wireless/scan.c b/net/wireless/scan.c index d4397eba5408..a32d52a04c27 100644 --- a/net/wireless/scan.c +++ b/net/wireless/scan.c @@ -161,7 +161,7 @@ static void __cfg80211_bss_expire(struct cfg80211_registered_device *dev, dev->bss_generation++; } -void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak) +void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev) { struct cfg80211_scan_request *request; struct wireless_dev *wdev; @@ -210,17 +210,7 @@ void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak) dev_put(wdev->netdev); rdev->scan_req = NULL; - - /* - * OK. If this is invoked with "leak" then we can't - * free this ... but we've cleaned it up anyway. The - * driver failed to call the scan_done callback, so - * all bets are off, it might still be trying to use - * the scan request or not ... if it accesses the dev - * in there (it shouldn't anyway) then it may crash. - */ - if (!leak) - kfree(request); + kfree(request); } void __cfg80211_scan_done(struct work_struct *wk) @@ -231,7 +221,7 @@ void __cfg80211_scan_done(struct work_struct *wk) scan_done_wk); rtnl_lock(); - ___cfg80211_scan_done(rdev, false); + ___cfg80211_scan_done(rdev); rtnl_unlock(); } |