mac80211: fix scan channel race
authorJohannes Berg <johannes@sipsolutions.net>
Thu, 7 May 2009 12:23:01 +0000 (14:23 +0200)
committerJohn W. Linville <linville@tuxdriver.com>
Mon, 11 May 2009 19:23:54 +0000 (15:23 -0400)
When a software scan starts, it first sets sw_scanning, but
leaves the scan_channel "unset" (it currently actually gets
initialised to a default). Now, when something else tries
to (re)configure the hardware in the window between these two
events (after sw_scanning = true, but before scan_channel is
set), the current code switches to the (unset!) scan_channel.
This causes trouble, especially when switching bands and
sending frames on the wrong channel.

To work around this, leave scan_channel initialised to NULL
and use it to determine whether or not a switch to a different
channel should occur (and also use the same condition to check
whether to adjust power for scan or not).

Additionally, avoid reconfiguring the hardware completely when
recalculating idle resulted in no changes, this was the problem
that originally led us to discover the race condition in the
first place, which was helpfully bisected by Pavel. This part
of the patch should not be necessary with the other fixes, but
not calling the ieee80211_hw_config function when we know it to
be unnecessary is certainly a correct thing to do.

Unfortunately, this patch cannot and does not fix the race
condition completely, but due to the way the scan code is
structured it makes the particular problem Pavel discovered
(race while changing channel at the same time as transmitting
frames) go away. To fix it completely, more work especially
with locking configuration is needed.

Bisected-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
net/mac80211/iface.c
net/mac80211/main.c
net/mac80211/scan.c

index 8b6daf0219f4bb3a0d99ed1a68a484fcda754cd3..8c9f1c722cdb2921a99c27f786f945e36aea750e 100644 (file)
@@ -964,5 +964,6 @@ void ieee80211_recalc_idle(struct ieee80211_local *local)
        mutex_lock(&local->iflist_mtx);
        chg = __ieee80211_recalc_idle(local);
        mutex_unlock(&local->iflist_mtx);
-       ieee80211_hw_config(local, chg);
+       if (chg)
+               ieee80211_hw_config(local, chg);
 }
index b80bc80e46cf9822417107128440b99955b21397..76df5eabf268e05c4369c06bff6cea45805b78c6 100644 (file)
@@ -154,15 +154,17 @@ static void ieee80211_master_set_multicast_list(struct net_device *dev)
 
 int ieee80211_hw_config(struct ieee80211_local *local, u32 changed)
 {
-       struct ieee80211_channel *chan;
+       struct ieee80211_channel *chan, *scan_chan;
        int ret = 0;
        int power;
        enum nl80211_channel_type channel_type;
 
        might_sleep();
 
-       if (local->sw_scanning) {
-               chan = local->scan_channel;
+       scan_chan = local->scan_channel;
+
+       if (scan_chan) {
+               chan = scan_chan;
                channel_type = NL80211_CHAN_NO_HT;
        } else {
                chan = local->oper_channel;
@@ -176,7 +178,7 @@ int ieee80211_hw_config(struct ieee80211_local *local, u32 changed)
                changed |= IEEE80211_CONF_CHANGE_CHANNEL;
        }
 
-       if (local->sw_scanning)
+       if (scan_chan)
                power = chan->max_power;
        else
                power = local->power_constr_level ?
@@ -859,8 +861,8 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
                if (!local->oper_channel) {
                        /* init channel we're on */
                        local->hw.conf.channel =
-                       local->oper_channel =
-                       local->scan_channel = &sband->channels[0];
+                       local->oper_channel = &sband->channels[0];
+                       local->hw.conf.channel_type = NL80211_CHAN_NO_HT;
                }
                channels += sband->n_channels;
 
index c99ef8d04d3df6fc0e79588eed87526a74a6f34e..e51b99b1473c6c7422ff7b39bca055be2d82741f 100644 (file)
@@ -298,6 +298,7 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw, bool aborted)
        was_hw_scan = local->hw_scanning;
        local->hw_scanning = false;
        local->sw_scanning = false;
+       local->scan_channel = NULL;
 
        /* we only have to protect scan_req and hw/sw scan */
        mutex_unlock(&local->scan_mtx);