On Mon, 2009-02-23 at 16:11 -0500, John W. Linville wrote:
> On Sat, Feb 21, 2009 at 08:33:06AM +0100, Lars Ericsson wrote:
> > Hi Johannes,
> >
> > I have discovered and patched a race in the scanning function since a couple
> > of releases.
> > To day I checked the current Linux git and the problem is still there.
> >
> > The problem is the sequence of events when the scan result is reported back.
> > The wireless_send_event(dev, SIOCGIWSCAN, &wrqu, NULL);
> > is called before ieee80211_hw_config(local);
> >
> > ieee80211_hw_config(local) will trig the wpa_supplicant to select an AP.
> > That may happen before the ieee80211_hw_config() is executed since the
> > wpa_supplicant
> > generated actions is executed by an other thread (wpa_supplicant).
> >
> > The result is that:
> > - wpa_supplicant setup for an association to an ap using correct channel.
> > - ieee80211_hw_config() reset the channel to the value before the SCAN
> > started.
> > - the association request will be sent out using the wrong channel.
> >
> >
> > Attached you will find the patch for 2.6.27.
> > It is not a perfect patch since the code is duplicated but it works :)
>
> Looks like the patch would need to be reworked -- the code in 2.6.29
> and later is different.
and the new code with cfg80211 is even more different ;)
> Also, any reason we can't just move the
> wireless_send_event() down to done:?
seems fine
> Still, this doesn't feel quite right. Shouldn't we be able to queue
> the userland-driven channel change until after the scan completes?
?
we do that, well, we set it here:
local->sw_scanning = false;
ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
but we claim to be on the user requested channel at all times, so that
should be ok
johannes
> John
>
> diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
> index f5c7c33..8c13a91 100644
> --- a/net/mac80211/scan.c
> +++ b/net/mac80211/scan.c
> @@ -437,15 +437,6 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
> local->last_scan_completed = jiffies;
> memset(&wrqu, 0, sizeof(wrqu));
>
> - /*
> - * 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->hw_scanning) {
> local->hw_scanning = false;
> /*
> @@ -486,6 +477,15 @@ void ieee80211_scan_completed(struct ieee80211_hw *hw)
> rcu_read_unlock();
>
> done:
> + /*
> + * 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);
> +
> ieee80211_mlme_notify_scan_completed(local);
> ieee80211_mesh_notify_scan_completed(local);
> }
>
Attachment:
signature.asc
Description: This is a digitally signed message part
