2009-06-11 10:54:25

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH] mac80211: Do not try to associate with an empty SSID

It looks like some programs (e.g., NM) are setting an empty SSID with
SIOCSIWESSID in some cases. This seems to trigger mac80211 to try to
associate with an invalid configuration (wildcard SSID) which will
result in failing associations (or odd issues, potentially including
kernel panic with some drivers) if the AP were to actually accept this
anyway).

Only start association process if the SSID is actually set. This
speeds up connection with NM in number of cases and avoids sending out
broken association request frames.

Signed-off-by: Jouni Malinen <[email protected]>

---
net/mac80211/mlme.c | 8 ++++++++
1 file changed, 8 insertions(+)

It is unclear when this was broken, but I think we need to get this into
2.6.31. The kernel panic (hopefully due to cleanup changes after 2.6.30)
can be worked around in the driver, I would assume, but it would be
better if we do not get into this odd state with associations since it
is not clear what various drivers will do with this kind of
configuration.


--- wireless-testing.orig/net/mac80211/mlme.c 2009-06-11 13:38:01.000000000 +0300
+++ wireless-testing/net/mac80211/mlme.c 2009-06-11 13:45:44.000000000 +0300
@@ -2436,6 +2436,14 @@ void ieee80211_sta_req_auth(struct ieee8
if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_STATION))
return;

+ if (ifmgd->ssid_len == 0) {
+ /*
+ * Only allow association to be started if a valid SSID is
+ * configured.
+ */
+ return;
+ }
+
if ((ifmgd->flags & (IEEE80211_STA_BSSID_SET |
IEEE80211_STA_AUTO_BSSID_SEL)) &&
(ifmgd->flags & (IEEE80211_STA_SSID_SET |

--
Jouni Malinen PGP id EFC895FA


2009-06-11 16:08:40

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: Do not try to associate with an empty SSID

On Thu, 2009-06-11 at 08:19 -0700, Jouni Malinen wrote:
> It looks like some programs (e.g., NM) are setting an empty SSID with
> SIOCSIWESSID in some cases. This seems to trigger mac80211 to try to
> associate with an invalid configuration (wildcard SSID) which will
> result in failing associations (or odd issues, potentially including
> kernel panic with some drivers) if the AP were to actually accept this
> anyway).

I think this behavior was recently changed in NM. See
http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=01a1bafc1d8abee153a8aca0495c77292f4f07e3

This is just fyi as this change is still needed for all the other NM
versions out there.

Reinette




2009-06-11 11:26:39

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Do not try to associate with an empty SSID

On Thu, 2009-06-11 at 13:54 +0300, Jouni Malinen wrote:
> It looks like some programs (e.g., NM) are setting an empty SSID with
> SIOCSIWESSID in some cases. This seems to trigger mac80211 to try to
> associate with an invalid configuration (wildcard SSID) which will
> result in failing associations (or odd issues, potentially including
> kernel panic with some drivers) if the AP were to actually accept this
> anyway).
>
> Only start association process if the SSID is actually set. This
> speeds up connection with NM in number of cases and avoids sending out
> broken association request frames.
>
> Signed-off-by: Jouni Malinen <[email protected]>
>
> ---
> net/mac80211/mlme.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> It is unclear when this was broken, but I think we need to get this into
> 2.6.31. The kernel panic (hopefully due to cleanup changes after 2.6.30)
> can be worked around in the driver, I would assume, but it would be
> better if we do not get into this odd state with associations since it
> is not clear what various drivers will do with this kind of
> configuration.
>
>
> --- wireless-testing.orig/net/mac80211/mlme.c 2009-06-11 13:38:01.000000000 +0300
> +++ wireless-testing/net/mac80211/mlme.c 2009-06-11 13:45:44.000000000 +0300
> @@ -2436,6 +2436,14 @@ void ieee80211_sta_req_auth(struct ieee8
> if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_STATION))
> return;
>
> + if (ifmgd->ssid_len == 0) {
> + /*
> + * Only allow association to be started if a valid SSID is
> + * configured.
> + */
> + return;
> + }
> +
> if ((ifmgd->flags & (IEEE80211_STA_BSSID_SET |
> IEEE80211_STA_AUTO_BSSID_SEL)) &&
> (ifmgd->flags & (IEEE80211_STA_SSID_SET |


This whole code is a little odd.. Shouldn't we disassoc if ssid_len ==
0? But then why don't we disassoc when the flags change etc? Looks like
this needs a good rewrite...

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-06-11 15:19:51

by Jouni Malinen

[permalink] [raw]
Subject: [PATCH v2] mac80211: Do not try to associate with an empty SSID

It looks like some programs (e.g., NM) are setting an empty SSID with
SIOCSIWESSID in some cases. This seems to trigger mac80211 to try to
associate with an invalid configuration (wildcard SSID) which will
result in failing associations (or odd issues, potentially including
kernel panic with some drivers) if the AP were to actually accept this
anyway).

Only start association process if the SSID is actually set. This
speeds up connection with NM in number of cases and avoids sending out
broken association request frames.

Signed-off-by: Jouni Malinen <[email protected]>

---
net/mac80211/mlme.c | 8 ++++++++
1 file changed, 8 insertions(+)

v2: Allow disassociation in ssid_len=0 case and only avoid starting a
new association. This function is difficult to understand and for now, I
just want to fix the critical issue here. Eventually, this code should
be redesigned or at least commented clearly.

--- wireless-testing.orig/net/mac80211/mlme.c 2009-06-11 13:38:01.000000000 +0300
+++ wireless-testing/net/mac80211/mlme.c 2009-06-11 18:08:39.000000000 +0300
@@ -2445,6 +2445,14 @@ void ieee80211_sta_req_auth(struct ieee8
ieee80211_set_disassoc(sdata, true, true,
WLAN_REASON_DEAUTH_LEAVING);

+ if (ifmgd->ssid_len == 0) {
+ /*
+ * Only allow association to be started if a valid SSID
+ * is configured.
+ */
+ return;
+ }
+
if (!(ifmgd->flags & IEEE80211_STA_EXT_SME) ||
ifmgd->state != IEEE80211_STA_MLME_ASSOCIATE)
set_bit(IEEE80211_STA_REQ_AUTH, &ifmgd->request);

--
Jouni Malinen PGP id EFC895FA

2009-06-11 15:27:34

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: Do not try to associate with an empty SSID

Jouni Malinen <[email protected]> writes:

> It looks like some programs (e.g., NM) are setting an empty SSID with
> SIOCSIWESSID in some cases. This seems to trigger mac80211 to try to
> associate with an invalid configuration (wildcard SSID) which will
> result in failing associations (or odd issues, potentially including
> kernel panic with some drivers) if the AP were to actually accept this
> anyway).
>
> Only start association process if the SSID is actually set. This
> speeds up connection with NM in number of cases and avoids sending out
> broken association request frames.
>
> Signed-off-by: Jouni Malinen <[email protected]>

Thanks, I think I have an internal bug report about a similar issue and
I hope this fixes that as well. But I need to investigate to be sure.

> v2: Allow disassociation in ssid_len=0 case and only avoid starting a
> new association. This function is difficult to understand and for now, I
> just want to fix the critical issue here. Eventually, this code should
> be redesigned or at least commented clearly.

Like we have talked earlier in this list, mlme.c requires a rewrite.

--
Kalle Valo