I have the problem when using 2.6.29 and judging by the code the situation
stays the same with wireless-testing, even after the fix "mac80211: deauth when
interface is marked down" (the fix doesn't work with kernel 2.6.28 or later, as
I'll explain).
The problem is, with latest kernels wpa_supplicant isn't still notified when
interface goes down, even after commit "mac80211: deauth when interface is
marked down" (e327b847 on Linus tree). There isn't a problem with this commit,
but because of other code changes it doesn't work on latest kernels (but works
if I apply same/similar change on 2.6.27 for example).
The issue is as follows: after commit "mac80211: restructure disassoc/deauth
flows" in 2.6.28, the call to ieee80211_sta_deauthenticate added by commit
e327b847 will not work: because we do sta_info_flush(local, sdata); inside
ieee80211_stop (iface.c), all stations in interface are cleared, so when calling
ieee80211_sta_deauthenticate->ieee80211_set_disassoc (mlme.c), inside
ieee80211_set_disassoc we have this in the beginning:
sta = sta_info_get(local, ifsta->bssid);
if (!sta) {
the !sta check triggers, so the function returns early and
ieee80211_sta_send_apinfo(sdata, ifsta); later isn't called, so wpa_supplicant
isn't notified with SIOCGIWAP
One possible fix is moving sta_info_flush, to avoid !sta, and seems good, like
the following:
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2acc416..0929581 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -370,25 +370,6 @@ static int ieee80211_stop(struct net_device *dev)
rcu_read_unlock();
/*
- * Remove all stations associated with this interface.
- *
- * This must be done before calling ops->remove_interface()
- * because otherwise we can later invoke ops->sta_notify()
- * whenever the STAs are removed, and that invalidates driver
- * assumptions about always getting a vif pointer that is valid
- * (because if we remove a STA after ops->remove_interface()
- * the driver will have removed the vif info already!)
- *
- * We could relax this and only unlink the stations from the
- * hash table and list but keep them on a per-sdata list that
- * will be inserted back again when the interface is brought
- * up again, but I don't currently see a use case for that,
- * except with WDS which gets a STA entry created when it is
- * brought up.
- */
- sta_info_flush(local, sdata);
-
- /*
* Don't count this interface for promisc/allmulti while it
* is down. dev_mc_unsync() will invoke set_multicast_list
* on the master interface which will sync these down to the
@@ -539,6 +520,26 @@ static int ieee80211_stop(struct net_device *dev)
conf.mac_addr = dev->dev_addr;
/* disable all keys for as long as this netdev is down */
ieee80211_disable_keys(sdata);
+
+ /*
+ * Remove all stations associated with this interface.
+ *
+ * This must be done before calling ops->remove_interface()
+ * because otherwise we can later invoke ops->sta_notify()
+ * whenever the STAs are removed, and that invalidates driver
+ * assumptions about always getting a vif pointer that is valid
+ * (because if we remove a STA after ops->remove_interface()
+ * the driver will have removed the vif info already!)
+ *
+ * We could relax this and only unlink the stations from the
+ * hash table and list but keep them on a per-sdata list that
+ * will be inserted back again when the interface is brought
+ * up again, but I don't currently see a use case for that,
+ * except with WDS which gets a STA entry created when it is
+ * brought up.
+ */
+ sta_info_flush(local, sdata);
+
local->ops->remove_interface(local_to_hw(local), &conf);
}
My question is, is the patch above safe? that is, do we need to always
call sta_info_flush on interface down like currently, or just for the case we
call remove_interface, like the comment states? if the latter, patch above
is good, and solves the wpa_supplicant/userspace issue.
--
[]'s
Herton.
On Mon, 2009-03-09 at 18:50 -0300, Herton Ronaldo Krzesinski wrote:
> I have the problem when using 2.6.29 and judging by the code the situation
> stays the same with wireless-testing, even after the fix "mac80211: deauth when
> interface is marked down" (the fix doesn't work with kernel 2.6.28 or later, as
> I'll explain).
>
> The problem is, with latest kernels wpa_supplicant isn't still notified when
> interface goes down, even after commit "mac80211: deauth when interface is
> marked down" (e327b847 on Linus tree). There isn't a problem with this commit,
> but because of other code changes it doesn't work on latest kernels (but works
> if I apply same/similar change on 2.6.27 for example).
>
> The issue is as follows: after commit "mac80211: restructure disassoc/deauth
> flows" in 2.6.28, the call to ieee80211_sta_deauthenticate added by commit
> e327b847 will not work: because we do sta_info_flush(local, sdata); inside
> ieee80211_stop (iface.c), all stations in interface are cleared, so when calling
> ieee80211_sta_deauthenticate->ieee80211_set_disassoc (mlme.c), inside
> ieee80211_set_disassoc we have this in the beginning:
>
> sta = sta_info_get(local, ifsta->bssid);
> if (!sta) {
>
> the !sta check triggers, so the function returns early and
> ieee80211_sta_send_apinfo(sdata, ifsta); later isn't called, so wpa_supplicant
> isn't notified with SIOCGIWAP
Interesting problem.
> One possible fix is moving sta_info_flush, to avoid !sta, and seems good, like
> the following:
>
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index 2acc416..0929581 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -370,25 +370,6 @@ static int ieee80211_stop(struct net_device *dev)
> rcu_read_unlock();
>
> /*
> - * Remove all stations associated with this interface.
> - *
> - * This must be done before calling ops->remove_interface()
> - * because otherwise we can later invoke ops->sta_notify()
> - * whenever the STAs are removed, and that invalidates driver
> - * assumptions about always getting a vif pointer that is valid
> - * (because if we remove a STA after ops->remove_interface()
> - * the driver will have removed the vif info already!)
> - *
> - * We could relax this and only unlink the stations from the
> - * hash table and list but keep them on a per-sdata list that
> - * will be inserted back again when the interface is brought
> - * up again, but I don't currently see a use case for that,
> - * except with WDS which gets a STA entry created when it is
> - * brought up.
> - */
> - sta_info_flush(local, sdata);
> -
> - /*
> * Don't count this interface for promisc/allmulti while it
> * is down. dev_mc_unsync() will invoke set_multicast_list
> * on the master interface which will sync these down to the
> @@ -539,6 +520,26 @@ static int ieee80211_stop(struct net_device *dev)
> conf.mac_addr = dev->dev_addr;
> /* disable all keys for as long as this netdev is down */
> ieee80211_disable_keys(sdata);
> +
> + /*
> + * Remove all stations associated with this interface.
> + *
> + * This must be done before calling ops->remove_interface()
> + * because otherwise we can later invoke ops->sta_notify()
> + * whenever the STAs are removed, and that invalidates driver
> + * assumptions about always getting a vif pointer that is valid
> + * (because if we remove a STA after ops->remove_interface()
> + * the driver will have removed the vif info already!)
> + *
> + * We could relax this and only unlink the stations from the
> + * hash table and list but keep them on a per-sdata list that
> + * will be inserted back again when the interface is brought
> + * up again, but I don't currently see a use case for that,
> + * except with WDS which gets a STA entry created when it is
> + * brought up.
> + */
> + sta_info_flush(local, sdata);
> +
> local->ops->remove_interface(local_to_hw(local), &conf);
> }
>
> My question is, is the patch above safe? that is, do we need to always
> call sta_info_flush on interface down like currently, or just for the case we
> call remove_interface, like the comment states? if the latter, patch above
> is good, and solves the wpa_supplicant/userspace issue.
The comment states it must be before remove_interface, not that it
doesn't need to be called unless that is called, and this code moving is
incorrect for AP_VLAN devices.
You could move the disassoc part out of the switch to before this.
johannes
Em Ter=E7a-feira 10 Mar=E7o 2009, =E0s 04:13:34, Johannes Berg escreveu=
:
> On Mon, 2009-03-09 at 18:50 -0300, Herton Ronaldo Krzesinski wrote:
> > I have the problem when using 2.6.29 and judging by the code the si=
tuation
> > stays the same with wireless-testing, even after the fix "mac80211:=
deauth when
> > interface is marked down" (the fix doesn't work with kernel 2.6.28 =
or later, as
> > I'll explain).
> >=20
> > The problem is, with latest kernels wpa_supplicant isn't still noti=
fied when
> > interface goes down, even after commit "mac80211: deauth when inter=
face is
> > marked down" (e327b847 on Linus tree). There isn't a problem with t=
his commit,
> > but because of other code changes it doesn't work on latest kernels=
(but works
> > if I apply same/similar change on 2.6.27 for example).
> >=20
> > The issue is as follows: after commit "mac80211: restructure disass=
oc/deauth
> > flows" in 2.6.28, the call to ieee80211_sta_deauthenticate added by=
commit
> > e327b847 will not work: because we do sta_info_flush(local, sdata);=
inside
> > ieee80211_stop (iface.c), all stations in interface are cleared, so=
when calling
> > ieee80211_sta_deauthenticate->ieee80211_set_disassoc (mlme.c), ins=
ide
> > ieee80211_set_disassoc we have this in the beginning:
> >=20
> > sta =3D sta_info_get(local, ifsta->bssid);
> > if (!sta) {
> >=20
> > the !sta check triggers, so the function returns early and
> > ieee80211_sta_send_apinfo(sdata, ifsta); later isn't called, so wpa=
_supplicant
> > isn't notified with SIOCGIWAP
>=20
> Interesting problem.
>=20
> > One possible fix is moving sta_info_flush, to avoid !sta, and seems=
good, like
> > the following:
> >=20
> > diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> > index 2acc416..0929581 100644
> > --- a/net/mac80211/iface.c
> > +++ b/net/mac80211/iface.c
> > @@ -370,25 +370,6 @@ static int ieee80211_stop(struct net_device *d=
ev)
> > rcu_read_unlock();
> > =20
> > /*
> > - * Remove all stations associated with this interface.
> > - *
> > - * This must be done before calling ops->remove_interface()
> > - * because otherwise we can later invoke ops->sta_notify()
> > - * whenever the STAs are removed, and that invalidates driver
> > - * assumptions about always getting a vif pointer that is valid
> > - * (because if we remove a STA after ops->remove_interface()
> > - * the driver will have removed the vif info already!)
> > - *
> > - * We could relax this and only unlink the stations from the
> > - * hash table and list but keep them on a per-sdata list that
> > - * will be inserted back again when the interface is brought
> > - * up again, but I don't currently see a use case for that,
> > - * except with WDS which gets a STA entry created when it is
> > - * brought up.
> > - */
> > - sta_info_flush(local, sdata);
> > -
> > - /*
> > * Don't count this interface for promisc/allmulti while it
> > * is down. dev_mc_unsync() will invoke set_multicast_list
> > * on the master interface which will sync these down to the
> > @@ -539,6 +520,26 @@ static int ieee80211_stop(struct net_device *d=
ev)
> > conf.mac_addr =3D dev->dev_addr;
> > /* disable all keys for as long as this netdev is down */
> > ieee80211_disable_keys(sdata);
> > +
> > + /*
> > + * Remove all stations associated with this interface.
> > + *
> > + * This must be done before calling ops->remove_interface()
> > + * because otherwise we can later invoke ops->sta_notify()
> > + * whenever the STAs are removed, and that invalidates driver
> > + * assumptions about always getting a vif pointer that is valid
> > + * (because if we remove a STA after ops->remove_interface()
> > + * the driver will have removed the vif info already!)
> > + *
> > + * We could relax this and only unlink the stations from the
> > + * hash table and list but keep them on a per-sdata list that
> > + * will be inserted back again when the interface is brought
> > + * up again, but I don't currently see a use case for that,
> > + * except with WDS which gets a STA entry created when it is
> > + * brought up.
> > + */
> > + sta_info_flush(local, sdata);
> > +
> > local->ops->remove_interface(local_to_hw(local), &conf);
> > }
> > =20
> > My question is, is the patch above safe? that is, do we need to alw=
ays
> > call sta_info_flush on interface down like currently, or just for t=
he case we
> > call remove_interface, like the comment states? if the latter, patc=
h above
> > is good, and solves the wpa_supplicant/userspace issue.
>=20
> The comment states it must be before remove_interface, not that it
> doesn't need to be called unless that is called, and this code moving=
is
> incorrect for AP_VLAN devices.
>=20
> You could move the disassoc part out of the switch to before this.
Ok, here is the new patch, if it's ok please apply (I can resubmit with=
patch
in subject/standalone message if necessary):
=46rom eb6f79597b9e6af8a3618873a781fd6ebcbd50a0 Mon Sep 17 00:00:00 200=
1
=46rom: Herton Ronaldo Krzesinski <[email protected]>
Date: Tue, 10 Mar 2009 09:44:17 -0300
Subject: [PATCH] mac80211: deauth before flushing STA information
Even after commit "mac80211: deauth when interface is marked down"
(e327b847 on Linus tree), userspace still isn't notified when interface
goes down. There isn't a problem with this commit, but because of other
code changes it doesn't work on kernels >=3D 2.6.28 (works if same/simi=
lar
change applied on 2.6.27 for example).
The issue is as follows: after commit "mac80211: restructure disassoc/d=
eauth
flows" in 2.6.28, the call to ieee80211_sta_deauthenticate added by
commit e327b847 will not work: because we do sta_info_flush(local, sdat=
a)
inside ieee80211_stop (iface.c), all stations in interface are cleared,=
so
when calling ieee80211_sta_deauthenticate->ieee80211_set_disassoc (mlme=
=2Ec),
inside ieee80211_set_disassoc we have this in the beginning:
sta =3D sta_info_get(local, ifsta->bssid);
if (!sta) {
The !sta check triggers, thus the function returns early and
ieee80211_sta_send_apinfo(sdata, ifsta) later isn't called, so
wpa_supplicant/userspace isn't notified with SIOCGIWAP.
This commit moves deauthentication to before flushing STA info
(sta_info_flush), thus the above can't happen and userspace is really
notified when interface goes down.
Signed-off-by: Herton Ronaldo Krzesinski <[email protected]>
---
net/mac80211/iface.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2acc416..f9f27b9 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -370,6 +370,18 @@ static int ieee80211_stop(struct net_device *dev)
rcu_read_unlock();
=20
/*
+ * Announce that we are leaving the network, in case we are a
+ * station interface type. This must be done before removing
+ * all stations associated with sta_info_flush, otherwise STA
+ * information will be gone and no announce being done.
+ */
+ if (sdata->vif.type =3D=3D NL80211_IFTYPE_STATION) {
+ if (sdata->u.mgd.state !=3D IEEE80211_STA_MLME_DISABLED)
+ ieee80211_sta_deauthenticate(sdata,
+ WLAN_REASON_DEAUTH_LEAVING);
+ }
+
+ /*
* Remove all stations associated with this interface.
*
* This must be done before calling ops->remove_interface()
@@ -454,10 +466,6 @@ static int ieee80211_stop(struct net_device *dev)
netif_addr_unlock_bh(local->mdev);
break;
case NL80211_IFTYPE_STATION:
- /* Announce that we are leaving the network. */
- if (sdata->u.mgd.state !=3D IEEE80211_STA_MLME_DISABLED)
- ieee80211_sta_deauthenticate(sdata,
- WLAN_REASON_DEAUTH_LEAVING);
memset(sdata->u.mgd.bssid, 0, ETH_ALEN);
del_timer_sync(&sdata->u.mgd.chswitch_timer);
del_timer_sync(&sdata->u.mgd.timer);
--=20
1.6.2
>=20
> johannes
>=20
--
[]'s
Herton