From: Johannes Berg <[email protected]>

When mac80211 changes the channel, it also calls into the driver's
bss_info_changed() callback, e.g. with BSS_CHANGED_IDLE. The driver
may, like iwlwifi does, access more data from bss_info in that case
and iwlwifi accesses the basic_rates bitmap, but if changing from a
band with more (basic) rates to one with fewer, an out-of-bounds
access of the rate array may result.

While we can't avoid having invalid data at some point in time, we
can avoid having it while we call the driver - so set up all the
data before configuring the channel, and then apply it afterwards.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=195677

Reported-by: Johannes Hirte <[email protected]>
Tested-by: Johannes Hirte <[email protected]>
Debugged-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
 net/mac80211/mlme.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 1ae9be090309..570d9ab61950 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4397,15 +4397,19 @@ static int ieee80211_prep_connection(struct 
ieee80211_sub_if_data *sdata,
                        return -ENOMEM;
        }
 
-       if (new_sta || override) {
-               err = ieee80211_prep_channel(sdata, cbss);
-               if (err) {
-                       if (new_sta)
-                               sta_info_free(local, new_sta);
-                       return -EINVAL;
-               }
-       }
-
+       /*
+        * Set up the information for the new channel before setting the
+        * new channel. We can't - completely race-free - change the basic
+        * rates bitmap and the channel (sband) that it refers to, but if
+        * we set it up before we at least avoid calling into the driver's
+        * bss_info_changed() method with invalid information (since we do
+        * call that from changing the channel - only for IDLE and perhaps
+        * some others, but ...).
+        *
+        * So to avoid that, just set up all the new information before the
+        * channel, but tell the driver to apply it only afterwards, since
+        * it might need the new channel for that.
+        */
        if (new_sta) {
                u32 rates = 0, basic_rates = 0;
                bool have_higher_than_11mbit;
@@ -4488,8 +4492,22 @@ static int ieee80211_prep_connection(struct 
ieee80211_sub_if_data *sdata,
                        sdata->vif.bss_conf.sync_dtim_count = 0;
                }
                rcu_read_unlock();
+       }
 
-               /* tell driver about BSSID, basic rates and timing */
+       if (new_sta || override) {
+               err = ieee80211_prep_channel(sdata, cbss);
+               if (err) {
+                       if (new_sta)
+                               sta_info_free(local, new_sta);
+                       return -EINVAL;
+               }
+       }
+
+       if (new_sta) {
+               /*
+                * tell driver about BSSID, basic rates and timing
+                * this was set up above, before setting the channel
+                */
                ieee80211_bss_info_change_notify(sdata,
                        BSS_CHANGED_BSSID | BSS_CHANGED_BASIC_RATES |
                        BSS_CHANGED_BEACON_INT);
-- 
2.11.0

Reply via email to