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

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 <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: Simon Raffeiner <[email protected]>
Cc: Andrey Yurovsky <[email protected]>
Cc: Javier Cardona <[email protected]>
---
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


2010-02-12 12:49:59

by Johannes Berg

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

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 <[email protected]>
> Cc: Johannes Berg <[email protected]>
> Cc: Simon Raffeiner <[email protected]>
> Cc: Andrey Yurovsky <[email protected]>
> Cc: Javier Cardona <[email protected]>
> ---
> 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

Attachments:
signature.asc (801.00 B)
This is a digitally signed message part
Subject: Re: [PATCH] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

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 <[email protected]>
> > Cc: Johannes Berg <[email protected]>
> > Cc: Simon Raffeiner <[email protected]>
> > Cc: Andrey Yurovsky <[email protected]>
> > Cc: Javier Cardona <[email protected]>
> > ---
> > 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.

Attachments:
(No filename) (1.81 kB)
 signature.asc (198.00 B)
Digital signature Download all attachments

2010-02-12 14:03:29

by Johannes Berg

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

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

Attachments:
signature.asc (801.00 B)
This is a digitally signed message part
Subject: [PATCH v2] nl80211: does not allow NEW_STATION and DEL_STATION for mesh

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 <[email protected]>
Cc: Johannes Berg <[email protected]>
Cc: Simon Raffeiner <[email protected]>
Cc: Andrey Yurovsky <[email protected]>
Cc: Javier Cardona <[email protected]>
---

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