diff mbox series

[09/13] cfg80211: Save the regulatory domain when setting custom regulatory

Message ID iwlwifi.20201129172929.290fa5c5568a.Ic5732aa64de6ee97ae3578bd5779fc723ba489d1@changeid
State New
Headers show
Series cfg80211/mac80211 patches from our internal tree 2020-11-29 | expand

Commit Message

Luca Coelho Nov. 29, 2020, 3:30 p.m. UTC 
From: Ilan Peer <ilan.peer@intel.com>

When custom regulatory was set, only the channels setting was updated, but
the regulatory domain was not saved. Fix it by saving it.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/reg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Marek Szyprowski Dec. 16, 2020, 10:20 a.m. UTC | #1 
Hi Luca,

On 29.11.2020 16:30, Luca Coelho wrote:
> From: Ilan Peer <ilan.peer@intel.com>

>

> When custom regulatory was set, only the channels setting was updated, but

> the regulatory domain was not saved. Fix it by saving it.

>

> Signed-off-by: Ilan Peer <ilan.peer@intel.com>

> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>


This patch landed recently in linux-next as commit beee24695157 
("cfg80211: Save the regulatory domain when setting custom regulatory"). 
It triggers the following warning on all boards I have, which use 
Broadcom chips. Here is an example from Raspberry Pi4:

cfg80211: Loading compiled-in X.509 certificates for regulatory database
cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'
cfg80211: loaded regulatory.db is malformed or signature is missing/invalid
brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43455-sdio for chip 
BCM4345/6
brcmfmac mmc1:0001:1: Direct firmware load for 
brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt failed with error -2
brcmfmac mmc1:0001:1: Falling back to sysfs fallback for: 
brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt
brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43455-sdio for chip 
BCM4345/6
brcmfmac: brcmf_c_process_clm_blob: no clm_blob available (err=-11), 
device may have limited channels available
brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM4345/6 wl0: Mar  1 2015 
07:29:38 version 7.45.18 (r538002) FWID 01-6a2c8ad4
Bluetooth: hci0: BCM: chip id 107
Bluetooth: hci0: BCM: features 0x2f
Bluetooth: hci0: BCM4345C0
Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000
Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch

=============================
WARNING: suspicious RCU usage
5.10.0-next-20201215+ #9962 Not tainted
-----------------------------
net/wireless/reg.c:144 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
2 locks held by kworker/1:1/32:
  #0: ffff000003405738 ((wq_completion)events){+.+.}-{0:0}, at: 
process_one_work+0x200/0x728
  #1: ffff80001321bdc0 ((work_completion)(&fw_work->work)){+.+.}-{0:0}, 
at: process_one_work+0x200/0x728

stack backtrace:
CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.10.0-next-20201215+ #9962
Hardware name: Raspberry Pi 4 Model B (DT)
Workqueue: events request_firmware_work_func
Call trace:
  dump_backtrace+0x0/0x1d0
  show_stack+0x14/0x60
  dump_stack+0xf4/0x15c
  lockdep_rcu_suspicious+0xd4/0xf8
  get_wiphy_regdom+0x6c/0x70 [cfg80211]
  wiphy_apply_custom_regulatory+0x80/0xc8 [cfg80211]
  brcmf_cfg80211_attach+0xb44/0x1330 [brcmfmac]
  brcmf_attach+0x174/0x4b8 [brcmfmac]
  brcmf_sdio_firmware_callback+0x670/0x7c8 [brcmfmac]
  brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
  request_firmware_work_func+0x4c/0xd8
  process_one_work+0x2a8/0x728
  worker_thread+0x48/0x460
  kthread+0x134/0x160
  ret_from_fork+0x10/0x18

Reverting this patch on top of linux next-20201215 hides this issue.

I compiled kernel for arm64 with defconfig with some additional options 
(I left only those that might be relevant to this issue):

./scripts/config --set-val CMA_SIZE_MBYTES 96 -e PROVE_LOCKING -e 
DEBUG_ATOMIC_SLEEP -e COMPILE_TEST -e PM_DEBUG -e PM_ADVANCED_DEBUG -e 
CONFIG_BRCMFMAC_PCIE

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Luca Coelho Dec. 17, 2020, 1:25 p.m. UTC | #2 
On Wed, 2020-12-16 at 11:20 +0100, Marek Szyprowski wrote:
> Hi Luca,


Hi Marek,


> On 29.11.2020 16:30, Luca Coelho wrote:

> > From: Ilan Peer <ilan.peer@intel.com>

> > 

> > When custom regulatory was set, only the channels setting was updated, but

> > the regulatory domain was not saved. Fix it by saving it.

> > 

> > Signed-off-by: Ilan Peer <ilan.peer@intel.com>

> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

> 

> This patch landed recently in linux-next as commit beee24695157 

> ("cfg80211: Save the regulatory domain when setting custom regulatory"). 

> It triggers the following warning on all boards I have, which use 

> Broadcom chips. Here is an example from Raspberry Pi4:

> 

> cfg80211: Loading compiled-in X.509 certificates for regulatory database

> cfg80211: Loaded X.509 cert 'sforshee: 00b28ddf47aef9cea7'

> cfg80211: loaded regulatory.db is malformed or signature is missing/invalid

> brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43455-sdio for chip 

> BCM4345/6

> brcmfmac mmc1:0001:1: Direct firmware load for 

> brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt failed with error -2

> brcmfmac mmc1:0001:1: Falling back to sysfs fallback for: 

> brcm/brcmfmac43455-sdio.raspberrypi,4-model-b.txt

> brcmfmac: brcmf_fw_alloc_request: using brcm/brcmfmac43455-sdio for chip 

> BCM4345/6

> brcmfmac: brcmf_c_process_clm_blob: no clm_blob available (err=-11), 

> device may have limited channels available

> brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM4345/6 wl0: Mar  1 2015 

> 07:29:38 version 7.45.18 (r538002) FWID 01-6a2c8ad4

> Bluetooth: hci0: BCM: chip id 107

> Bluetooth: hci0: BCM: features 0x2f

> Bluetooth: hci0: BCM4345C0

> Bluetooth: hci0: BCM4345C0 (003.001.025) build 0000

> Bluetooth: hci0: BCM4345C0 'brcm/BCM4345C0.hcd' Patch

> 

> =============================

> WARNING: suspicious RCU usage

> 5.10.0-next-20201215+ #9962 Not tainted

> -----------------------------

> net/wireless/reg.c:144 suspicious rcu_dereference_check() usage!

> 

> other info that might help us debug this:

> 

> 

> rcu_scheduler_active = 2, debug_locks = 1

> 2 locks held by kworker/1:1/32:

>   #0: ffff000003405738 ((wq_completion)events){+.+.}-{0:0}, at: 

> process_one_work+0x200/0x728

>   #1: ffff80001321bdc0 ((work_completion)(&fw_work->work)){+.+.}-{0:0}, 

> at: process_one_work+0x200/0x728

> 

> stack backtrace:

> CPU: 1 PID: 32 Comm: kworker/1:1 Not tainted 5.10.0-next-20201215+ #9962

> Hardware name: Raspberry Pi 4 Model B (DT)

> Workqueue: events request_firmware_work_func

> Call trace:

>   dump_backtrace+0x0/0x1d0

>   show_stack+0x14/0x60

>   dump_stack+0xf4/0x15c

>   lockdep_rcu_suspicious+0xd4/0xf8

>   get_wiphy_regdom+0x6c/0x70 [cfg80211]

>   wiphy_apply_custom_regulatory+0x80/0xc8 [cfg80211]

>   brcmf_cfg80211_attach+0xb44/0x1330 [brcmfmac]

>   brcmf_attach+0x174/0x4b8 [brcmfmac]

>   brcmf_sdio_firmware_callback+0x670/0x7c8 [brcmfmac]

>   brcmf_fw_request_done+0x7c/0x100 [brcmfmac]

>   request_firmware_work_func+0x4c/0xd8

>   process_one_work+0x2a8/0x728

>   worker_thread+0x48/0x460

>   kthread+0x134/0x160

>   ret_from_fork+0x10/0x18

> 

> Reverting this patch on top of linux next-20201215 hides this issue.


This is indeed an issue.  Now syzbot also reported it.  We currently
have an issue with our test machinery that is not enabling lockdep and
other lock checks...

We'll fix this asap.

--
Cheers,
Luca.
Nicolas Cavallari June 15, 2021, 2:42 p.m. UTC | #3 
On 29/11/2020 16:30, Luca Coelho wrote:
> From: Ilan Peer <ilan.peer@intel.com>

> 

> When custom regulatory was set, only the channels setting was updated, but

> the regulatory domain was not saved. Fix it by saving it.

> 

> Signed-off-by: Ilan Peer <ilan.peer@intel.com>

> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

> ---

>   net/wireless/reg.c | 8 ++++++++

>   1 file changed, 8 insertions(+)

> 

> diff --git a/net/wireless/reg.c b/net/wireless/reg.c

> index a04fdfb35f07..094492b62f8a 100644

> --- a/net/wireless/reg.c

> +++ b/net/wireless/reg.c

> @@ -2547,6 +2547,7 @@ static void handle_band_custom(struct wiphy *wiphy,

>   void wiphy_apply_custom_regulatory(struct wiphy *wiphy,

>   				   const struct ieee80211_regdomain *regd)

>   {

> +	const struct ieee80211_regdomain *new_regd, *tmp;

>   	enum nl80211_band band;

>   	unsigned int bands_set = 0;

>   

> @@ -2566,6 +2567,13 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,

>   	 * on your device's supported bands.

>   	 */

>   	WARN_ON(!bands_set);

> +	new_regd = reg_copy_regd(regd);

> +	if (IS_ERR(new_regd))

> +		return;

> +

> +	tmp = get_wiphy_regdom(wiphy);

> +	rcu_assign_pointer(wiphy->regd, new_regd);

> +	rcu_free_regdom(tmp);

>   }

>   EXPORT_SYMBOL(wiphy_apply_custom_regulatory);

>   

> 


Hello,

This patch somehow appears to break ath9k's eeprom hints and restrict it to the 
world regulatory domain on v5.12.10.

ath9k calls wiphy_apply_custom_regulatory() with its own kind of world 
regulatory domain, before it decodes hints from the eeprom and uses 
regulatory_hint() to request a specific alpha2.

With this patch, applying the hint fails because wiphy->regd is already set.
If i revert this patch, ath9k works again.
Kalle Valo Oct. 11, 2021, 10:51 a.m. UTC | #4 
Nicolas Cavallari <Nicolas.Cavallari@green-communications.fr> writes:

> On 29/11/2020 16:30, Luca Coelho wrote:

>> From: Ilan Peer <ilan.peer@intel.com>

>>

>> When custom regulatory was set, only the channels setting was updated, but

>> the regulatory domain was not saved. Fix it by saving it.

>>

>> Signed-off-by: Ilan Peer <ilan.peer@intel.com>

>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

>> ---

>>   net/wireless/reg.c | 8 ++++++++

>>   1 file changed, 8 insertions(+)

>>

>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c

>> index a04fdfb35f07..094492b62f8a 100644

>> --- a/net/wireless/reg.c

>> +++ b/net/wireless/reg.c

>> @@ -2547,6 +2547,7 @@ static void handle_band_custom(struct wiphy *wiphy,

>>   void wiphy_apply_custom_regulatory(struct wiphy *wiphy,

>>   				   const struct ieee80211_regdomain *regd)

>>   {

>> +	const struct ieee80211_regdomain *new_regd, *tmp;

>>   	enum nl80211_band band;

>>   	unsigned int bands_set = 0;

>>   @@ -2566,6 +2567,13 @@ void wiphy_apply_custom_regulatory(struct

>> wiphy *wiphy,

>>   	 * on your device's supported bands.

>>   	 */

>>   	WARN_ON(!bands_set);

>> +	new_regd = reg_copy_regd(regd);

>> +	if (IS_ERR(new_regd))

>> +		return;

>> +

>> +	tmp = get_wiphy_regdom(wiphy);

>> +	rcu_assign_pointer(wiphy->regd, new_regd);

>> +	rcu_free_regdom(tmp);

>>   }

>>   EXPORT_SYMBOL(wiphy_apply_custom_regulatory);

>>   

>>

>

> Hello,

>

> This patch somehow appears to break ath9k's eeprom hints and restrict

> it to the world regulatory domain on v5.12.10.

>

> ath9k calls wiphy_apply_custom_regulatory() with its own kind of world

> regulatory domain, before it decodes hints from the eeprom and uses

> regulatory_hint() to request a specific alpha2.

>

> With this patch, applying the hint fails because wiphy->regd is already set.

> If i revert this patch, ath9k works again.


I have lost track, is this regression fixed now or is it sill
unresolved?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Nicolas Cavallari Oct. 11, 2021, 2:09 p.m. UTC | #5 
On 11/10/2021 12:51, Kalle Valo wrote:
> Nicolas Cavallari <Nicolas.Cavallari@green-communications.fr> writes:

> 

>> On 29/11/2020 16:30, Luca Coelho wrote:

>>> From: Ilan Peer <ilan.peer@intel.com>

>>>

>>> When custom regulatory was set, only the channels setting was updated, but

>>> the regulatory domain was not saved. Fix it by saving it.

>>>

>>> Signed-off-by: Ilan Peer <ilan.peer@intel.com>

>>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

>>> ---

>>>    net/wireless/reg.c | 8 ++++++++

>>>    1 file changed, 8 insertions(+)

>>>

>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c

>>> index a04fdfb35f07..094492b62f8a 100644

>>> --- a/net/wireless/reg.c

>>> +++ b/net/wireless/reg.c

>>> @@ -2547,6 +2547,7 @@ static void handle_band_custom(struct wiphy *wiphy,

>>>    void wiphy_apply_custom_regulatory(struct wiphy *wiphy,

>>>    				   const struct ieee80211_regdomain *regd)

>>>    {

>>> +	const struct ieee80211_regdomain *new_regd, *tmp;

>>>    	enum nl80211_band band;

>>>    	unsigned int bands_set = 0;

>>>    @@ -2566,6 +2567,13 @@ void wiphy_apply_custom_regulatory(struct

>>> wiphy *wiphy,

>>>    	 * on your device's supported bands.

>>>    	 */

>>>    	WARN_ON(!bands_set);

>>> +	new_regd = reg_copy_regd(regd);

>>> +	if (IS_ERR(new_regd))

>>> +		return;

>>> +

>>> +	tmp = get_wiphy_regdom(wiphy);

>>> +	rcu_assign_pointer(wiphy->regd, new_regd);

>>> +	rcu_free_regdom(tmp);

>>>    }

>>>    EXPORT_SYMBOL(wiphy_apply_custom_regulatory);

>>>    

>>>

>>

>> Hello,

>>

>> This patch somehow appears to break ath9k's eeprom hints and restrict

>> it to the world regulatory domain on v5.12.10.

>>

>> ath9k calls wiphy_apply_custom_regulatory() with its own kind of world

>> regulatory domain, before it decodes hints from the eeprom and uses

>> regulatory_hint() to request a specific alpha2.

>>

>> With this patch, applying the hint fails because wiphy->regd is already set.

>> If i revert this patch, ath9k works again.

> 

> I have lost track, is this regression fixed now or is it sill

> unresolved?


I admit i forgot about it after reverting the patch and haven't tried a new 
kernel with ath9k since then, but from a quick glance, the code hasn't changed. 
I'll try a new kernel tomorrow.
Johannes Berg Feb. 3, 2022, 10:30 p.m. UTC | #6 
On Tue, 2021-10-12 at 12:24 +0200, Nicolas Cavallari wrote:
> > > 
> > > This patch somehow appears to break ath9k's eeprom hints and restrict
> > > it to the world regulatory domain on v5.12.10.
> > > 
> > > ath9k calls wiphy_apply_custom_regulatory() with its own kind of world
> > > regulatory domain, before it decodes hints from the eeprom and uses
> > > regulatory_hint() to request a specific alpha2.
> > > 
> > > With this patch, applying the hint fails because wiphy->regd is already set.
> > > If i revert this patch, ath9k works again.
> > 

Hm. It stands to reason that perhaps ath9k should call
wiphy_apply_custom_regulatory(NULL) (if that's possible) to reset the
knowledge of having a custom regulatory domain, before requesting the
correct one be applied by cfg80211 (and possibly even crda userspace).

I can't really say that I think the patch itself is wrong, even if it
caused this problem.

johannes
Nicolas Cavallari Feb. 7, 2022, 5:56 p.m. UTC | #7 
On 03/02/2022 23:30, Johannes Berg wrote:
> On Tue, 2021-10-12 at 12:24 +0200, Nicolas Cavallari wrote:
>>>>
>>>> This patch somehow appears to break ath9k's eeprom hints and restrict
>>>> it to the world regulatory domain on v5.12.10.
>>>>
>>>> ath9k calls wiphy_apply_custom_regulatory() with its own kind of world
>>>> regulatory domain, before it decodes hints from the eeprom and uses
>>>> regulatory_hint() to request a specific alpha2.
>>>>
>>>> With this patch, applying the hint fails because wiphy->regd is already set.
>>>> If i revert this patch, ath9k works again.
>>>
> 
> Hm. It stands to reason that perhaps ath9k should call
> wiphy_apply_custom_regulatory(NULL) (if that's possible) to reset the
> knowledge of having a custom regulatory domain, before requesting the
> correct one be applied by cfg80211 (and possibly even crda userspace).

wiphy_apply_custom_regulatory(NULL) is apparently not possible.
The regulatory code is still a black box to me...
diff mbox series

Patch

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index a04fdfb35f07..094492b62f8a 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2547,6 +2547,7 @@  static void handle_band_custom(struct wiphy *wiphy,
 void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 				   const struct ieee80211_regdomain *regd)
 {
+	const struct ieee80211_regdomain *new_regd, *tmp;
 	enum nl80211_band band;
 	unsigned int bands_set = 0;
 
@@ -2566,6 +2567,13 @@  void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 	 * on your device's supported bands.
 	 */
 	WARN_ON(!bands_set);
+	new_regd = reg_copy_regd(regd);
+	if (IS_ERR(new_regd))
+		return;
+
+	tmp = get_wiphy_regdom(wiphy);
+	rcu_assign_pointer(wiphy->regd, new_regd);
+	rcu_free_regdom(tmp);
 }
 EXPORT_SYMBOL(wiphy_apply_custom_regulatory);