[PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[Thadeu Lima de Souza Cascardo]

As discussed in linux-wireless mailint list, adding and removing
stations for mesh topologies is not necessary. Since doing it triggers
bugs, the sugestion was to simply disable it.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Simon Raffeiner <sturmflut@lieberbiber.de>
Cc: Andrey Yurovsky <andrey@cozybit.com>
Cc: Javier Cardona <javier@cozybit.com>
---
 net/wireless/nl80211.c |   47 +++++++++++------------------------------------
 1 files changed, 11 insertions(+), 36 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5b79ecf..176b8fe 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2001,6 +2001,10 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 
 	memset(&params, 0, sizeof(params));
 
+	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN)
+		return -EINVAL;
+
 	if (!info->attrs[NL80211_ATTR_MAC])
 		return -EINVAL;
 
@@ -2010,6 +2014,9 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	if (!info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES])
 		return -EINVAL;
 
+	if (!info->attrs[NL80211_ATTR_STA_AID])
+		return -EINVAL;
+
 	mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
 	params.supported_rates =
 		nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
@@ -2018,11 +2025,9 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	params.listen_interval =
 		nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
 
-	if (info->attrs[NL80211_ATTR_STA_AID]) {
-		params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
-		if (!params.aid || params.aid > IEEE80211_MAX_AID)
-			return -EINVAL;
-	}
+	params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
+	if (!params.aid || params.aid > IEEE80211_MAX_AID)
+		return -EINVAL;
 
 	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
 		params.ht_capa =
@@ -2044,35 +2049,6 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	/* validate settings */
 	err = 0;
 
-	switch (dev->ieee80211_ptr->iftype) {
-	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_AP_VLAN:
-		/* all ok but must have AID */
-		if (!params.aid)
-			err = -EINVAL;
-		break;
-	case NL80211_IFTYPE_MESH_POINT:
-		/* disallow things mesh doesn't support */
-		if (params.vlan)
-			err = -EINVAL;
-		if (params.aid)
-			err = -EINVAL;
-		if (params.ht_capa)
-			err = -EINVAL;
-		if (params.listen_interval >= 0)
-			err = -EINVAL;
-		if (params.supported_rates)
-			err = -EINVAL;
-		if (params.sta_flags_mask)
-			err = -EINVAL;
-		break;
-	default:
-		err = -EINVAL;
-	}
-
-	if (err)
-		goto out;
-
 	if (!rdev->ops->add_station) {
 		err = -EOPNOTSUPP;
 		goto out;
@@ -2113,8 +2089,7 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
 		goto out_rtnl;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
-	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN &&
-	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_MESH_POINT) {
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN) {
 		err = -EINVAL;
 		goto out;
 	}
-- 
1.6.6.1

Re: [PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

On Fri, 2010-02-12 at 10:37 -0200, Thadeu Lima de Souza Cascardo wrote:
> As discussed in linux-wireless mailint list, adding and removing
> stations for mesh topologies is not necessary. Since doing it triggers
> bugs, the sugestion was to simply disable it.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: Simon Raffeiner <sturmflut@lieberbiber.de>
> Cc: Andrey Yurovsky <andrey@cozybit.com>
> Cc: Javier Cardona <javier@cozybit.com>
> ---
>  net/wireless/nl80211.c |   47
> +++++++++++------------------------------------
>  1 files changed, 11 insertions(+), 36 deletions(-)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 5b79ecf..176b8fe 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2001,6 +2001,10 @@ static int nl80211_new_station(struct sk_buff
> *skb, struct genl_info *info)
>  
>  	memset(&params, 0, sizeof(params));
>  
> +	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
> +	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN)
> +		return -EINVAL;
> +
>  	if (!info->attrs[NL80211_ATTR_MAC])
>  		return -EINVAL;

You clearly didn't ever try this code.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[Thadeu Lima de Souza Cascardo]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

On Fri, Feb 12, 2010 at 01:49:53PM +0100, Johannes Berg wrote:
> On Fri, 2010-02-12 at 10:37 -0200, Thadeu Lima de Souza Cascardo wrote:
> > As discussed in linux-wireless mailint list, adding and removing
> > stations for mesh topologies is not necessary. Since doing it triggers
> > bugs, the sugestion was to simply disable it.
> > 
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > Cc: Johannes Berg <johannes@sipsolutions.net>
> > Cc: Simon Raffeiner <sturmflut@lieberbiber.de>
> > Cc: Andrey Yurovsky <andrey@cozybit.com>
> > Cc: Javier Cardona <javier@cozybit.com>
> > ---
> >  net/wireless/nl80211.c |   47
> > +++++++++++------------------------------------
> >  1 files changed, 11 insertions(+), 36 deletions(-)
> > 
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 5b79ecf..176b8fe 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -2001,6 +2001,10 @@ static int nl80211_new_station(struct sk_buff
> > *skb, struct genl_info *info)
> >  
> >  	memset(&params, 0, sizeof(params));
> >  
> > +	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
> > +	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN)
> > +		return -EINVAL;
> > +
> >  	if (!info->attrs[NL80211_ATTR_MAC])
> >  		return -EINVAL;
> 
> You clearly didn't ever try this code.
> 
> johannes

Sorry. This code has obviously given me an oops. I did it in a hurry and
thought that was a good optimization to get out as early as possible in
the case the device was not in AP mode.

I have fixed it, but I would like to do a more thorough test now to make
up for my mistake.

So, is there any test tool around? I am playing with iw code to do
these tests. Any recommendations for the values of listen interval and
the acceptable values for supported rates and aid?

Regards,
Cascardo.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

On Fri, 2010-02-12 at 11:55 -0200, Thadeu Lima de Souza Cascardo wrote:

> Sorry. This code has obviously given me an oops. I did it in a hurry
> and
> thought that was a good optimization to get out as early as possible
> in
> the case the device was not in AP mode.

Which device, at that point :)

> I have fixed it, but I would like to do a more thorough test now to
> make
> up for my mistake.
> 
> So, is there any test tool around? I am playing with iw code to do
> these tests. Any recommendations for the values of listen interval and
> the acceptable values for supported rates and aid?

I guess just using hostapd and connecting to it once would be a good
test for that code path, but the rest _looked_ fine to me.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[PATCH v2] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

[Thadeu Lima de Souza Cascardo]

As discussed in linux-wireless mailing list, adding and removing
stations for mesh topologies is not necessary. Since doing it triggers
bugs, the sugestion was to simply disable it.

Tested using a custom iw command "station new". Works only after using
hostapd. "station del" command also works.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Simon Raffeiner <sturmflut@lieberbiber.de>
Cc: Andrey Yurovsky <andrey@cozybit.com>
Cc: Javier Cardona <javier@cozybit.com>
---

Should be fixed now! Sorry for all the noise.

---
 net/wireless/nl80211.c |   49 ++++++++++++-----------------------------------
 1 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5b79ecf..a95ab9e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2010,6 +2010,9 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	if (!info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES])
 		return -EINVAL;
 
+	if (!info->attrs[NL80211_ATTR_STA_AID])
+		return -EINVAL;
+
 	mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
 	params.supported_rates =
 		nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
@@ -2018,11 +2021,9 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	params.listen_interval =
 		nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
 
-	if (info->attrs[NL80211_ATTR_STA_AID]) {
-		params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
-		if (!params.aid || params.aid > IEEE80211_MAX_AID)
-			return -EINVAL;
-	}
+	params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
+	if (!params.aid || params.aid > IEEE80211_MAX_AID)
+		return -EINVAL;
 
 	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
 		params.ht_capa =
@@ -2037,6 +2038,12 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		goto out_rtnl;
 
+	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	err = get_vlan(info, rdev, &params.vlan);
 	if (err)
 		goto out;
@@ -2044,35 +2051,6 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	/* validate settings */
 	err = 0;
 
-	switch (dev->ieee80211_ptr->iftype) {
-	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_AP_VLAN:
-		/* all ok but must have AID */
-		if (!params.aid)
-			err = -EINVAL;
-		break;
-	case NL80211_IFTYPE_MESH_POINT:
-		/* disallow things mesh doesn't support */
-		if (params.vlan)
-			err = -EINVAL;
-		if (params.aid)
-			err = -EINVAL;
-		if (params.ht_capa)
-			err = -EINVAL;
-		if (params.listen_interval >= 0)
-			err = -EINVAL;
-		if (params.supported_rates)
-			err = -EINVAL;
-		if (params.sta_flags_mask)
-			err = -EINVAL;
-		break;
-	default:
-		err = -EINVAL;
-	}
-
-	if (err)
-		goto out;
-
 	if (!rdev->ops->add_station) {
 		err = -EOPNOTSUPP;
 		goto out;
@@ -2113,8 +2091,7 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
 		goto out_rtnl;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
-	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN &&
-	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_MESH_POINT) {
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP_VLAN) {
 		err = -EINVAL;
 		goto out;
 	}
-- 
1.6.6.1