2018-07-03 23:04:29

by Peter Oh

[permalink] [raw]
Subject: [PATCH] cfg80211: inspect off channel operation only when off channel given

From: Peter Oh <[email protected]>

NL80211_ATTR_OFFCHANNEL_TX_OK does not mean given channel is always
off channel, but it means the channel given could be off channel.
Hence it should not block the given channel to be used if given
channel does not require off channel mgmt tx although regulatory
domain is non-ETSI.

Signed-off-by: Peter Oh <[email protected]>
---
net/wireless/nl80211.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4eece06..991042b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -9915,7 +9915,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;

wdev_lock(wdev);
- if (params.offchan && !cfg80211_off_channel_oper_allowed(wdev)) {
+ if (params.offchan &&
+ !cfg80211_chandef_identical(&chandef, &wdev->chandef) &&
+ !cfg80211_off_channel_oper_allowed(wdev)) {
wdev_unlock(wdev);
return -EBUSY;
}
--
2.7.4


2018-07-06 12:31:38

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: inspect off channel operation only when off channel given

On Tue, 2018-07-03 at 16:04 -0700, [email protected] wrote:
> From: Peter Oh <[email protected]>
>
> NL80211_ATTR_OFFCHANNEL_TX_OK does not mean given channel is always
> off channel, but it means the channel given could be off channel.
> Hence it should not block the given channel to be used if given
> channel does not require off channel mgmt tx although regulatory
> domain is non-ETSI.
>
> Signed-off-by: Peter Oh <[email protected]>
> ---
> net/wireless/nl80211.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 4eece06..991042b 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -9915,7 +9915,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
> return -EINVAL;
>
> wdev_lock(wdev);
> - if (params.offchan && !cfg80211_off_channel_oper_allowed(wdev)) {
> + if (params.offchan &&
> + !cfg80211_chandef_identical(&chandef, &wdev->chandef) &&
> + !cfg80211_off_channel_oper_allowed(wdev)) {
> wdev_unlock(wdev);

Hmm. That seems fine, but can we be sure that wdev->chandef is always
valid? ISTR that it isn't necessarily updated all the time, but I can't
really say right now.

johannes

2019-08-30 10:40:46

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: inspect off channel operation only when off channel given

On Fri, 2018-07-06 at 14:31 +0200, Johannes Berg wrote:
> On Tue, 2018-07-03 at 16:04 -0700, [email protected] wrote:
> > From: Peter Oh <[email protected]>
> >
> > NL80211_ATTR_OFFCHANNEL_TX_OK does not mean given channel is always
> > off channel, but it means the channel given could be off channel.
> > Hence it should not block the given channel to be used if given
> > channel does not require off channel mgmt tx although regulatory
> > domain is non-ETSI.
> >
> > Signed-off-by: Peter Oh <[email protected]>
> > ---
> > net/wireless/nl80211.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 4eece06..991042b 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -9915,7 +9915,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
> > return -EINVAL;
> >
> > wdev_lock(wdev);
> > - if (params.offchan && !cfg80211_off_channel_oper_allowed(wdev)) {
> > + if (params.offchan &&
> > + !cfg80211_chandef_identical(&chandef, &wdev->chandef) &&
> > + !cfg80211_off_channel_oper_allowed(wdev)) {
> > wdev_unlock(wdev);
>
> Hmm. That seems fine, but can we be sure that wdev->chandef is always
> valid? ISTR that it isn't necessarily updated all the time, but I can't
> really say right now.

For the record, in addition to this question, the commit log might need
some rewording since the whole regulatory/non-ETSI part isn't really
obvious (and not clear to me right now).

I've had this patch waiting for about a year now, I'll drop it. Please
resend if it's still relevant.

johannes