2011-11-22 18:10:13

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 0/3] cfg80211: few regulatory fixes

Here's my queue of fixes for cfg80211 regulatory. One is a performance
fix so whether or not it goes, I'll leave it to you guys. I document it
very well though. The others are fixes that should go to stable. One of them
is based on an issue I found through the shiny new regulatory simulator, but
also helps address the issue Johannes has found. The last patch amends
Johannes' proposed fix by using the new fix I found through the regulatory
simulator.

Unfortunately I haven't had time to test these myself.. so just spitting
them all out for review.

Luis R. Rodriguez (3):
cfg80211: allow following country IE power for custom regdom cards
cfg80211: fix race on init and driver registration
cfg80211: amend regulatory NULL dereference fix

include/net/cfg80211.h | 4 ++-
net/wireless/reg.c | 50 +++++++++++++++++++++++++++++++++++------------
2 files changed, 40 insertions(+), 14 deletions(-)

--
1.7.4.15.g7811d



2011-11-22 18:10:29

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 2/3] cfg80211: fix race on init and driver registration

There is a theoretical race that if hit will trigger
a crash. The race is between when we issue the first
regulatory hint, regulatory_hint_core(), gets processed
by the workqueue and between when the first device
gets registered to the wireless core. This is not easy
to reproduce but it was easy to do so through the
regulatory simulator I have been working on. This
is a port of the fix I implemented there [1].

[1] https://github.com/mcgrof/regsim/commit/a246ccf81f059cb662eee288aa13100f631e4cc8

Cc: [email protected]
Cc: Johannes Berg <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
net/wireless/reg.c | 28 ++++++++++++++++++++--------
1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index d4865a0..c8c3c59 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -57,8 +57,17 @@
#define REG_DBG_PRINT(args...)
#endif

+static struct regulatory_request core_request_world = {
+ .initiator = NL80211_REGDOM_SET_BY_CORE,
+ .alpha2[0] = '0',
+ .alpha2[1] = '0',
+ .intersect = false,
+ .processed = true,
+ .country_ie_env = ENVIRON_ANY,
+};
+
/* Receipt of information from last regulatory request */
-static struct regulatory_request *last_request;
+static struct regulatory_request *last_request = &core_request_world;

/* To trigger userspace events */
static struct platform_device *reg_pdev;
@@ -165,6 +174,10 @@ static void reset_regdomains(void)

cfg80211_world_regdom = &world_regdom;
cfg80211_regdomain = NULL;
+
+ if (last_request != &core_request_world)
+ kfree(last_request);
+ last_request = &core_request_world;
}

/*
@@ -1421,7 +1434,8 @@ static int __regulatory_hint(struct wiphy *wiphy,
}

new_request:
- kfree(last_request);
+ if (last_request != &core_request_world)
+ kfree(last_request);

last_request = pending_request;
last_request->intersect = intersect;
@@ -1591,9 +1605,6 @@ static int regulatory_hint_core(const char *alpha2)
{
struct regulatory_request *request;

- kfree(last_request);
- last_request = NULL;
-
request = kzalloc(sizeof(struct regulatory_request),
GFP_KERNEL);
if (!request)
@@ -1835,6 +1846,10 @@ static void restore_regulatory_settings(bool reset_user)
/* First restore to the basic regulatory settings */
cfg80211_regdomain = cfg80211_world_regdom;

+ if (last_request != &core_request_world)
+ kfree(last_request);
+ last_request = &core_request_world;
+
mutex_unlock(&reg_mutex);
mutex_unlock(&cfg80211_mutex);

@@ -2318,9 +2333,6 @@ void /* __init_or_exit */ regulatory_exit(void)

reset_regdomains();

- kfree(last_request);
-
- last_request = NULL;
dev_set_uevent_suppress(&reg_pdev->dev, true);

platform_device_unregister(reg_pdev);
--
1.7.4.15.g7811d


2011-11-23 14:27:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH 1/3] cfg80211: allow following country IE power for custom regdom cards

On Wed, Nov 23, 2011 at 2:07 AM, Rajkumar Manoharan
<[email protected]> wrote:
> On Tue, Nov 22, 2011 at 10:10:00AM -0800, Luis R. Rodriguez wrote:
>> By definition WIPHY_FLAG_STRICT_REGULATORY was intended to allow the
>> wiphy to adjust itself to the country IE power information if the
>> card had no regulatory data but we had no way to tell cfg80211 that if
>> the card also had its own custom regulatory domain (these are typically
>> custom world regulatory domains) that we want to follow the country IE's
>> noted values for power for each channel. We add support for this and
>> document it.
>>
>> This is not a critical fix but a performance optimization for cards
>> with custom regulatory domains that associate to an AP with sends
>> out country IEs with a higher EIRP than the one on the custom
>> regulatory domain. In practice the only driver affected right now
>> are the Atheros drivers as they are the only drivers using both
>> WIPHY_FLAG_STRICT_REGULATORY and WIPHY_FLAG_CUSTOM_REGULATORY --
>> used on cards that have an Atheros world regulatory domain. Cards
>> that have been programmed to follow a country specifically will not
>> follow the country IE power. So although not a stable fix distributions
>> should consider cherry picking this.
>>
> Luis,
>
> After some testing, I noticed that the incresed power value seems to be
> remained after the disconnection. So while falling back to world
> regulatory domain the band updations are ignored at ignore_reg_update.
> If the driver follows STRICT_REGULATORY, the it should not be ignored.
> isn't it?

I'll look into this, thanks for testing. John please hold off on these
patches, I will respin them along with my stable fixes.

Luis

2011-11-22 18:10:40

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 3/3] cfg80211: amend regulatory NULL dereference fix

Johanne's patch for "cfg80211: fix regulatory NULL dereference"
was fine but needed some other set of fixes which my previous
patches addresses. This one ammends his fix with the other
considerations applied.

Cc: [email protected]
Cc: Johannes Berg <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
net/wireless/reg.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index c8c3c59..b3393d6 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2104,7 +2104,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)

request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
if (!request_wiphy) {
- reg_set_request_processed();
+ schedule_delayed_work(&reg_timeout, 0);
return -ENODEV;
}

--
1.7.4.15.g7811d


2011-11-23 10:07:28

by Rajkumar Manoharan

[permalink] [raw]
Subject: Re: [PATCH 1/3] cfg80211: allow following country IE power for custom regdom cards

On Tue, Nov 22, 2011 at 10:10:00AM -0800, Luis R. Rodriguez wrote:
> By definition WIPHY_FLAG_STRICT_REGULATORY was intended to allow the
> wiphy to adjust itself to the country IE power information if the
> card had no regulatory data but we had no way to tell cfg80211 that if
> the card also had its own custom regulatory domain (these are typically
> custom world regulatory domains) that we want to follow the country IE's
> noted values for power for each channel. We add support for this and
> document it.
>
> This is not a critical fix but a performance optimization for cards
> with custom regulatory domains that associate to an AP with sends
> out country IEs with a higher EIRP than the one on the custom
> regulatory domain. In practice the only driver affected right now
> are the Atheros drivers as they are the only drivers using both
> WIPHY_FLAG_STRICT_REGULATORY and WIPHY_FLAG_CUSTOM_REGULATORY --
> used on cards that have an Atheros world regulatory domain. Cards
> that have been programmed to follow a country specifically will not
> follow the country IE power. So although not a stable fix distributions
> should consider cherry picking this.
>
Luis,

After some testing, I noticed that the incresed power value seems to be
remained after the disconnection. So while falling back to world
regulatory domain the band updations are ignored at ignore_reg_update.
If the driver follows STRICT_REGULATORY, the it should not be ignored.
isn't it?

--
Rajkumar


2011-11-22 18:10:20

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH 1/3] cfg80211: allow following country IE power for custom regdom cards

By definition WIPHY_FLAG_STRICT_REGULATORY was intended to allow the
wiphy to adjust itself to the country IE power information if the
card had no regulatory data but we had no way to tell cfg80211 that if
the card also had its own custom regulatory domain (these are typically
custom world regulatory domains) that we want to follow the country IE's
noted values for power for each channel. We add support for this and
document it.

This is not a critical fix but a performance optimization for cards
with custom regulatory domains that associate to an AP with sends
out country IEs with a higher EIRP than the one on the custom
regulatory domain. In practice the only driver affected right now
are the Atheros drivers as they are the only drivers using both
WIPHY_FLAG_STRICT_REGULATORY and WIPHY_FLAG_CUSTOM_REGULATORY --
used on cards that have an Atheros world regulatory domain. Cards
that have been programmed to follow a country specifically will not
follow the country IE power. So although not a stable fix distributions
should consider cherry picking this.

Cc: [email protected]
Cc: Paul Stewart <[email protected]>
Cc: Rajkumar Manoharan <[email protected]>
Cc: Senthilkumar Balasubramanian <[email protected]>
Reported-by: Rajkumar Manoharan <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
---

I will apply this to the stable series of compat-wireless. Whether or
not you guys want this for stable.. is your call ;) I want it for my
stable releases :D

include/net/cfg80211.h | 4 +++-
net/wireless/reg.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d5e1891..698d443 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1683,7 +1683,9 @@ struct cfg80211_ops {
* regulatory domain no user regulatory domain can enable these channels
* at a later time. This can be used for devices which do not have
* calibration information guaranteed for frequencies or settings
- * outside of its regulatory domain.
+ * outside of its regulatory domain. If used in combination with
+ * WIPHY_FLAG_CUSTOM_REGULATORY the inspected country IE power settings
+ * will be followed.
* @WIPHY_FLAG_DISABLE_BEACON_HINTS: enable this if your driver needs to ensure
* that passive scan flags and beaconing flags may not be lifted by
* cfg80211 due to regulatory beacon hints. For more information on beacon
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 76b35df..d4865a0 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -857,10 +857,22 @@ static void handle_channel(struct wiphy *wiphy,
chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
chan->max_antenna_gain = min(chan->orig_mag,
(int) MBI_TO_DBI(power_rule->max_antenna_gain));
- if (chan->orig_mpwr)
- chan->max_power = min(chan->orig_mpwr,
- (int) MBM_TO_DBM(power_rule->max_eirp));
- else
+ if (chan->orig_mpwr) {
+ /*
+ * Devices that have their own custom regulatory domain
+ * but also use WIPHY_FLAG_STRICT_REGULATORY will follow the
+ * passed country IE power settings.
+ */
+ if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+ wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY &&
+ wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) {
+ chan->max_power =
+ MBM_TO_DBM(power_rule->max_eirp);
+ } else {
+ chan->max_power = min(chan->orig_mpwr,
+ (int) MBM_TO_DBM(power_rule->max_eirp));
+ }
+ } else
chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp);
}

--
1.7.4.15.g7811d