[PATCH] cfg80211: check lost scans later, fix bug

* [PATCH] cfg80211: check lost scans later, fix bug
@ 2009-08-20 18:46 Johannes Berg
  2009-08-20 19:36 ` [PATCH v2] " Johannes Berg
  0 siblings, 1 reply; 2+ messages in thread
From: Johannes Berg @ 2009-08-20 18:46 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless, Felix Fietkau

When we lose a scan, cfg80211 tries to clean up after
the driver. However, it currently does this too early,
it does this in GOING_DOWN already instead of DOWN, so
it may happen with mac80211. Besides fixing this, also
make it more robust by leaking the scan request so if
the driver later actually finishes the scan, it won't
crash. Also check in ___cfg80211_scan_done whether a
scan request is still pending and exit if not.

Reported-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Tested-by: Felix Fietkau <nbd@openwrt.org>
---
 net/wireless/core.c |    4 +++-
 net/wireless/core.h |    2 +-
 net/wireless/scan.c |   21 ++++++++++++++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

--- wireless-testing.orig/net/wireless/core.c	2009-08-20 20:29:10.000000000 +0200
+++ wireless-testing/net/wireless/core.c	2009-08-20 20:29:12.000000000 +0200
@@ -662,7 +662,7 @@ static void wdev_cleanup_work(struct wor
 
 	if (WARN_ON(rdev->scan_req && rdev->scan_req->dev == wdev->netdev)) {
 		rdev->scan_req->aborted = true;
-		___cfg80211_scan_done(rdev);
+		___cfg80211_scan_done(rdev, true);
 	}
 
 	cfg80211_unlock_rdev(rdev);
@@ -753,6 +753,8 @@ static int cfg80211_netdev_notifier_call
 		default:
 			break;
 		}
+		break;
+	case NETDEV_DOWN:
 		dev_hold(dev);
 		schedule_work(&wdev->cleanup_work);
 		break;
--- wireless-testing.orig/net/wireless/core.h	2009-08-20 20:29:10.000000000 +0200
+++ wireless-testing/net/wireless/core.h	2009-08-20 20:29:12.000000000 +0200
@@ -370,7 +370,7 @@ void cfg80211_sme_scan_done(struct net_d
 void cfg80211_sme_rx_auth(struct net_device *dev, const u8 *buf, size_t len);
 void cfg80211_sme_disassoc(struct net_device *dev, int idx);
 void __cfg80211_scan_done(struct work_struct *wk);
-void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev);
+void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak);
 void cfg80211_upload_connect_keys(struct wireless_dev *wdev);
 
 struct ieee80211_channel *
--- wireless-testing.orig/net/wireless/scan.c	2009-08-20 20:29:10.000000000 +0200
+++ wireless-testing/net/wireless/scan.c	2009-08-20 20:35:13.000000000 +0200
@@ -18,7 +18,7 @@
 
 #define IEEE80211_SCAN_RESULT_EXPIRE	(15 * HZ)
 
-void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev)
+void ___cfg80211_scan_done(struct cfg80211_registered_device *rdev, bool leak)
 {
 	struct cfg80211_scan_request *request;
 	struct net_device *dev;
@@ -26,8 +26,13 @@ void ___cfg80211_scan_done(struct cfg802
 	union iwreq_data wrqu;
 #endif
 
+	ASSERT_RDEV_LOCK();
+
 	request = rdev->scan_req;
 
+	if (!request)
+		return;
+
 	dev = request->dev;
 
 	/*
@@ -53,7 +58,17 @@ void ___cfg80211_scan_done(struct cfg802
 	dev_put(dev);
 
 	rdev->scan_req = NULL;
-	kfree(request);
+
+	/*
+	 * 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);
 }
 
 void __cfg80211_scan_done(struct work_struct *wk)
@@ -64,7 +79,7 @@ void __cfg80211_scan_done(struct work_st
 			    scan_done_wk);
 
 	cfg80211_lock_rdev(rdev);
-	___cfg80211_scan_done(rdev);
+	___cfg80211_scan_done(rdev, false);
 	cfg80211_unlock_rdev(rdev);
 }
 



^ permalink raw reply	[flat|nested] 2+ messages in thread