mac80211: fix scan vs. interface removal race

When we remove an interface, we can currently end up having
a pointer to it left in local->scan_sdata after it has been
set down, and then with a hardware scan the scan completion
can try to access it which is a bug. Alternatively, a scan
that started as a hardware scan may terminate as though it
was a software scan, if the timing is just right.

On SMP systems, software scan also has a similar problem,
just canceling the delayed work and setting a flag isn't
enough since it may be running concurrently; in this case
we would also never restore state of other interfaces.

This patch hopefully fixes the problems by always invoking
ieee80211_scan_completed or requiring it to be invoked by
the driver, I suspect the drivers that have ->hw_scan() are
buggy. The bug will not manifest itself unless you remove
the interface while hw-scanning which will also turn off
the hw, and then add a new interface which will be unusable
until you scan once.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

1 files changed, 27 insertions, 11 deletions

diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index f4399e9ac928..27727027d76d 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -430,9 +430,20 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
         struct ieee80211_sub_if_data *sdata;
         union iwreq_data wrqu;
 
+        if (WARN_ON(!local->sta_hw_scanning && !local->sta_sw_scanning))
+                return;
+
         local->last_scan_completed = jiffies;
         memset(&wrqu, 0, sizeof(wrqu));
-        wireless_send_event(local->scan_sdata->dev, SIOCGIWSCAN, &wrqu, NULL);
+
+        /*
+         * local->scan_sdata could have been NULLed by the interface
+         * down code in case we were scanning on an interface that is
+         * being taken down.
+         */
+        sdata = local->scan_sdata;
+        if (sdata)
+                wireless_send_event(sdata->dev, SIOCGIWSCAN, &wrqu, NULL);
 
         if (local->sta_hw_scanning) {
                 local->sta_hw_scanning = 0;
@@ -491,7 +502,10 @@ void ieee80211_sta_scan_work(struct work_struct *work)
         int skip;
         unsigned long next_delay = 0;
 
-        if (!local->sta_sw_scanning)
+        /*
+         * Avoid re-scheduling when the sdata is going away.
+         */
+        if (!netif_running(sdata->dev))
                 return;
 
         switch (local->scan_state) {
@@ -570,9 +584,8 @@ void ieee80211_sta_scan_work(struct work_struct *work)
                 break;
         }
 
-        if (local->sta_sw_scanning)
-                queue_delayed_work(local->hw.workqueue, &local->scan_work,
-                                   next_delay);
+        queue_delayed_work(local->hw.workqueue, &local->scan_work,
+                           next_delay);
 }
 
 
@@ -609,13 +622,16 @@ int ieee80211_sta_start_scan(struct ieee80211_sub_if_data *scan_sdata,
         }
 
         if (local->ops->hw_scan) {
-                int rc = local->ops->hw_scan(local_to_hw(local),
-                                             ssid, ssid_len);
-                if (!rc) {
-                        local->sta_hw_scanning = 1;
-                        local->scan_sdata = scan_sdata;
+                int rc;
+
+                local->sta_hw_scanning = 1;
+                rc = local->ops->hw_scan(local_to_hw(local), ssid, ssid_len);
+                if (rc) {
+                        local->sta_hw_scanning = 0;
+                        return rc;
                 }
-                return rc;
+                local->scan_sdata = scan_sdata;
+                return 0;
         }
 
         local->sta_sw_scanning = 1;