Re: [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel in non-ETSI

[Thiagarajan, Vasanthakumar]

On Thursday 26 January 2017 03:06 PM, Johannes Berg wrote:
>
>> +static bool cfg80211_off_channel_oper_allowed(struct wireless_dev
>> *wdev)
>> +{
>> +if (!cfg80211_beaconing_iface_active(wdev))
>> +return true;
>> +
>> +if (!(wdev->chandef.chan->flags & IEEE80211_CHAN_RADAR))
>> +return true;
>
> That could use some locking assertions. Maybe also in the
> cfg80211_beaconing_iface_active() function you introduced in the
> previous patch.

Ok.

>
>> +if (!cfg80211_off_channel_oper_allowed(wdev)) {
>> +struct ieee80211_channel *chan;
>> +
>> +if (request->n_channels != 1) {
>> +err = -EBUSY;
>> +goto out_free;
>> +}
>> +
>> +chan = request->channels[0];
>> +if (chan->center_freq != wdev->chandef.chan-
>>> center_freq) {
>> +err = -EBUSY;
>> +goto out_free;
>> +}
>> +}
>
> I'm not convinced you even hold the relevant locks here, though off the
> top of my head I'm not even sure which are needed.

Thanks for pointing it, it should be within wdev_lock().

>
>>  i = 0;
>>  if (n_ssids) {
>>  nla_for_each_nested(attr, info-
>>> attrs[NL80211_ATTR_SCAN_SSIDS], tmp) {
>> @@ -9053,6 +9079,7 @@ static int nl80211_remain_on_channel(struct
>> sk_buff *skb,
>>  struct cfg80211_registered_device *rdev = info->user_ptr[0];
>>  struct wireless_dev *wdev = info->user_ptr[1];
>>  struct cfg80211_chan_def chandef;
>> +const struct cfg80211_chan_def *compat_chandef;
>>  struct sk_buff *msg;
>>  void *hdr;
>>  u64 cookie;
>> @@ -9081,6 +9108,14 @@ static int nl80211_remain_on_channel(struct
>> sk_buff *skb,
>>  if (err)
>>  return err;
>>
>> +if (!(cfg80211_off_channel_oper_allowed(wdev) ||
>> +  cfg80211_chandef_identical(>chandef, )))
>
> I'd prefer to write that as !off_channel && !chandef_identical, seems
> easier to understand here.

Ok.


Thanks,

Vasanth

Re: [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel in non-ETSI

[Thiagarajan, Vasanthakumar]

On Wednesday 25 January 2017 11:50 PM, Jean-Pierre Tosoni wrote:
>> -Message d'origine-
>> De : linux-wireless-ow...@vger.kernel.org [mailto:linux-wireless-
>> ow...@vger.kernel.org] De la part de Vasanthakumar Thiagarajan
>> Envoyé : mercredi 25 janvier 2017 12:31
>> À : johan...@sipsolutions.net
>> Cc : linux-wireless@vger.kernel.org; Vasanthakumar Thiagarajan
>> Objet : [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel
>> in non-ETSI
>>
>> For non-ETSI regulatory domain, CAC result on DFS channel may not be valid
>> once moving out of that channel (as done during remain-on-channel,
>> scannning and off-channel tx).
>> Running CAC on an operating DFS channel after every off-channel operation
>> will only add complexity and disturb the current link. Better do not allow
>> any off-channel switch from a DFS operating channel in non-ETSI domain.
>>
>
> Moving out should be forbidden only to "master" devices i.e. AP, mesh,
> ad-hoc.
> "Slave" devices i.e. client stations, are not responsible for detecting
> radars, hence, they can do an off-channel scan and go back to a "DFS
> operating channel" without waiting for CAC.
>
> It looks like your patch would forbid multichannel scan ?
>
>

No, this patch forbids off-channel switch only for DFS master device.
cfg80211_beaconing_iface_active() checks if it is a beaconing interface.
I'll mention this in the commit log.

Vasanth

Re: [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel in non-ETSI

[Johannes Berg]

> +static bool cfg80211_off_channel_oper_allowed(struct wireless_dev
> *wdev)
> +{
> + if (!cfg80211_beaconing_iface_active(wdev))
> + return true;
> +
> + if (!(wdev->chandef.chan->flags & IEEE80211_CHAN_RADAR))
> + return true;

That could use some locking assertions. Maybe also in the
cfg80211_beaconing_iface_active() function you introduced in the
previous patch.

> + if (!cfg80211_off_channel_oper_allowed(wdev)) {
> + struct ieee80211_channel *chan;
> +
> + if (request->n_channels != 1) {
> + err = -EBUSY;
> + goto out_free;
> + }
> +
> + chan = request->channels[0];
> + if (chan->center_freq != wdev->chandef.chan-
> >center_freq) {
> + err = -EBUSY;
> + goto out_free;
> + }
> + }

I'm not convinced you even hold the relevant locks here, though off the
top of my head I'm not even sure which are needed.

>   i = 0;
>   if (n_ssids) {
>   nla_for_each_nested(attr, info-
> >attrs[NL80211_ATTR_SCAN_SSIDS], tmp) {
> @@ -9053,6 +9079,7 @@ static int nl80211_remain_on_channel(struct
> sk_buff *skb,
>   struct cfg80211_registered_device *rdev = info->user_ptr[0];
>   struct wireless_dev *wdev = info->user_ptr[1];
>   struct cfg80211_chan_def chandef;
> + const struct cfg80211_chan_def *compat_chandef;
>   struct sk_buff *msg;
>   void *hdr;
>   u64 cookie;
> @@ -9081,6 +9108,14 @@ static int nl80211_remain_on_channel(struct
> sk_buff *skb,
>   if (err)
>   return err;
>  
> + if (!(cfg80211_off_channel_oper_allowed(wdev) ||
> +   cfg80211_chandef_identical(>chandef, )))

I'd prefer to write that as !off_channel && !chandef_identical, seems
easier to understand here.

johannes

RE: [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel in non-ETSI

[Jean-Pierre Tosoni]

> -Message d'origine-
> De : linux-wireless-ow...@vger.kernel.org [mailto:linux-wireless-
> ow...@vger.kernel.org] De la part de Vasanthakumar Thiagarajan
> Envoyé : mercredi 25 janvier 2017 12:31
> À : johan...@sipsolutions.net
> Cc : linux-wireless@vger.kernel.org; Vasanthakumar Thiagarajan
> Objet : [RFC 2/3] cfg80211: Disallow moving out of operating DFS channel
> in non-ETSI
> 
> For non-ETSI regulatory domain, CAC result on DFS channel may not be valid
> once moving out of that channel (as done during remain-on-channel,
> scannning and off-channel tx).
> Running CAC on an operating DFS channel after every off-channel operation
> will only add complexity and disturb the current link. Better do not allow
> any off-channel switch from a DFS operating channel in non-ETSI domain.
> 

Moving out should be forbidden only to "master" devices i.e. AP, mesh,
ad-hoc.
"Slave" devices i.e. client stations, are not responsible for detecting
radars, hence, they can do an off-channel scan and go back to a "DFS
operating channel" without waiting for CAC.

It looks like your patch would forbid multichannel scan ?



> Signed-off-by: Vasanthakumar Thiagarajan <vthia...@qti.qualcomm.com>
> ---
>  net/wireless/nl80211.c | 38 ++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index
> 63dfa60..c614af4 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -6506,6 +6506,17 @@ static int nl80211_parse_random_mac(struct nlattr
> **attrs,
>   return 0;
>  }
> 
> +static bool cfg80211_off_channel_oper_allowed(struct wireless_dev
> +*wdev) {
> + if (!cfg80211_beaconing_iface_active(wdev))
> + return true;
> +
> + if (!(wdev->chandef.chan->flags & IEEE80211_CHAN_RADAR))
> + return true;
> +
> + return regulatory_pre_cac_allowed(wdev->wiphy);
> +}
> +
>  static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info
> *info)  {
>   struct cfg80211_registered_device *rdev = info->user_ptr[0]; @@ -
> 6631,6 +6642,21 @@ static int nl80211_trigger_scan(struct sk_buff *skb,
> struct genl_info *info)
> 
>   request->n_channels = i;
> 
> + if (!cfg80211_off_channel_oper_allowed(wdev)) {
> + struct ieee80211_channel *chan;
> +
> + if (request->n_channels != 1) {
> + err = -EBUSY;
> + goto out_free;
> + }
> +
> + chan = request->channels[0];
> + if (chan->center_freq != wdev->chandef.chan->center_freq) {
> + err = -EBUSY;
> + goto out_free;
> + }
> + }
> +
>   i = 0;
>   if (n_ssids) {
>   nla_for_each_nested(attr, info-
> >attrs[NL80211_ATTR_SCAN_SSIDS], tmp) { @@ -9053,6 +9079,7 @@ static int
> nl80211_remain_on_channel(struct sk_buff *skb,
>   struct cfg80211_registered_device *rdev = info->user_ptr[0];
>   struct wireless_dev *wdev = info->user_ptr[1];
>   struct cfg80211_chan_def chandef;
> + const struct cfg80211_chan_def *compat_chandef;
>   struct sk_buff *msg;
>   void *hdr;
>   u64 cookie;
> @@ -9081,6 +9108,14 @@ static int nl80211_remain_on_channel(struct sk_buff
> *skb,
>   if (err)
>   return err;
> 
> + if (!(cfg80211_off_channel_oper_allowed(wdev) ||
> +   cfg80211_chandef_identical(>chandef, ))) {
> + compat_chandef = cfg80211_chandef_compatible(>chandef,
> +  );
> + if (compat_chandef != )
> + return -EBUSY;
> + }
> +
>   msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>   if (!msg)
>   return -ENOMEM;
> @@ -9256,6 +9291,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb,
> struct genl_info *info)
>   if (!chandef.chan && params.offchan)
>   return -EINVAL;
> 
> + if (params.offchan && !cfg80211_off_channel_oper_allowed(wdev))
> + return -EBUSY;
> +
>   params.buf = nla_data(info->attrs[NL80211_ATTR_FRAME]);
>   params.len = nla_len(info->attrs[NL80211_ATTR_FRAME]);
> 
> --
> 1.9.1

[RFC 2/3] cfg80211: Disallow moving out of operating DFS channel in non-ETSI

[Vasanthakumar Thiagarajan]

For non-ETSI regulatory domain, CAC result on DFS channel
may not be valid once moving out of that channel (as done
during remain-on-channel, scannning and off-channel tx).
Running CAC on an operating DFS channel after every off-channel
operation will only add complexity and disturb the current
link. Better do not allow any off-channel switch from a DFS
operating channel in non-ETSI domain.

Signed-off-by: Vasanthakumar Thiagarajan 
---
 net/wireless/nl80211.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 63dfa60..c614af4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6506,6 +6506,17 @@ static int nl80211_parse_random_mac(struct nlattr 
**attrs,
return 0;
 }
 
+static bool cfg80211_off_channel_oper_allowed(struct wireless_dev *wdev)
+{
+   if (!cfg80211_beaconing_iface_active(wdev))
+   return true;
+
+   if (!(wdev->chandef.chan->flags & IEEE80211_CHAN_RADAR))
+   return true;
+
+   return regulatory_pre_cac_allowed(wdev->wiphy);
+}
+
 static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 {
struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -6631,6 +6642,21 @@ static int nl80211_trigger_scan(struct sk_buff *skb, 
struct genl_info *info)
 
request->n_channels = i;
 
+   if (!cfg80211_off_channel_oper_allowed(wdev)) {
+   struct ieee80211_channel *chan;
+
+   if (request->n_channels != 1) {
+   err = -EBUSY;
+   goto out_free;
+   }
+
+   chan = request->channels[0];
+   if (chan->center_freq != wdev->chandef.chan->center_freq) {
+   err = -EBUSY;
+   goto out_free;
+   }
+   }
+
i = 0;
if (n_ssids) {
nla_for_each_nested(attr, info->attrs[NL80211_ATTR_SCAN_SSIDS], 
tmp) {
@@ -9053,6 +9079,7 @@ static int nl80211_remain_on_channel(struct sk_buff *skb,
struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct wireless_dev *wdev = info->user_ptr[1];
struct cfg80211_chan_def chandef;
+   const struct cfg80211_chan_def *compat_chandef;
struct sk_buff *msg;
void *hdr;
u64 cookie;
@@ -9081,6 +9108,14 @@ static int nl80211_remain_on_channel(struct sk_buff *skb,
if (err)
return err;
 
+   if (!(cfg80211_off_channel_oper_allowed(wdev) ||
+ cfg80211_chandef_identical(>chandef, ))) {
+   compat_chandef = cfg80211_chandef_compatible(>chandef,
+);
+   if (compat_chandef != )
+   return -EBUSY;
+   }
+
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!msg)
return -ENOMEM;
@@ -9256,6 +9291,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct 
genl_info *info)
if (!chandef.chan && params.offchan)
return -EINVAL;
 
+   if (params.offchan && !cfg80211_off_channel_oper_allowed(wdev))
+   return -EBUSY;
+
params.buf = nla_data(info->attrs[NL80211_ATTR_FRAME]);
params.len = nla_len(info->attrs[NL80211_ATTR_FRAME]);
 
-- 
1.9.1