[PATCH 0/2] cfg80211: two reg fixes

[PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This rebases Arik's patch to apply first and then piggy back
the non modular fix on top of that. I think you all have
more fancy things to test this series than I do now.

Michael Leun <lkml20140418@newton.leun.net> you reported a regression
with commit 2a901468c221e778af52603e006a53 on v3.14, I however cannot
see how that commit caused the issue you reported, can you test this
patch ported onto v3.14? A port is here:

http://drvbp1.linux-foundation.org/~mcgrof/patches/2014/04/22/test-ariks-patch-for-314.patch

Arik Nemtsov (1):
  cfg80211: avoid freeing last_request while in flight

Luis R. Rodriguez (1):
  cfg80211: fix processing world regdomain when non modular

 net/wireless/reg.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

-- 
1.9.0

[PATCH 1/2] cfg80211: avoid freeing last_request while in flight

[Luis R. Rodriguez]

From: Arik Nemtsov <arik@wizery.com>

Avoid freeing the last request while it is being processed. This can
happen in some cases if reg_work is kicked for some reason while the
currently pending request is in flight.

Cc: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Arik Nemtsov <arik@wizery.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 net/wireless/reg.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 9d32633..081c571 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -263,8 +263,16 @@ static char user_alpha2[2];
 module_param(ieee80211_regdom, charp, 0444);
 MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
-static void reg_free_request(struct regulatory_request *lr)
+static void reg_free_request(struct regulatory_request *request)
 {
+	if (request != get_last_request())
+		kfree(request);
+}
+
+static void reg_free_last_request(void)
+{
+	struct regulatory_request *lr = get_last_request();
+
 	if (lr != &core_request_world && lr)
 		kfree_rcu(lr, rcu_head);
 }
@@ -277,7 +285,7 @@ static void reg_update_last_request(struct regulatory_request *request)
 	if (lr == request)
 		return;
 
-	reg_free_request(lr);
+	reg_free_last_request();
 	rcu_assign_pointer(last_request, request);
 }
 
@@ -1661,7 +1669,7 @@ reg_process_hint_user(struct regulatory_request *user_request)
 	if (treatment == REG_REQ_IGNORE ||
 	    treatment == REG_REQ_ALREADY_SET ||
 	    treatment == REG_REQ_USER_HINT_HANDLED) {
-		kfree(user_request);
+		reg_free_request(user_request);
 		return treatment;
 	}
 
@@ -1722,14 +1730,14 @@ reg_process_hint_driver(struct wiphy *wiphy,
 		break;
 	case REG_REQ_IGNORE:
 	case REG_REQ_USER_HINT_HANDLED:
-		kfree(driver_request);
+		reg_free_request(driver_request);
 		return treatment;
 	case REG_REQ_INTERSECT:
 		/* fall through */
 	case REG_REQ_ALREADY_SET:
 		regd = reg_copy_regd(get_cfg80211_regdom());
 		if (IS_ERR(regd)) {
-			kfree(driver_request);
+			reg_free_request(driver_request);
 			return REG_REQ_IGNORE;
 		}
 		rcu_assign_pointer(wiphy->regd, regd);
@@ -1824,10 +1832,10 @@ reg_process_hint_country_ie(struct wiphy *wiphy,
 	case REG_REQ_USER_HINT_HANDLED:
 		/* fall through */
 	case REG_REQ_ALREADY_SET:
-		kfree(country_ie_request);
+		reg_free_request(country_ie_request);
 		return treatment;
 	case REG_REQ_INTERSECT:
-		kfree(country_ie_request);
+		reg_free_request(country_ie_request);
 		/*
 		 * This doesn't happen yet, not sure we
 		 * ever want to support it for this case.
@@ -1888,7 +1896,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 	return;
 
 out_free:
-	kfree(reg_request);
+	reg_free_request(reg_request);
 }
 
 /*
-- 
1.9.0

[PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This allows processing of the last regulatory request when
we determine its still pending. Without this if a regulatory
request failed to get processed by userspace we wouldn't
be able to re-process it later. An example situation that can
lead to an unprocessed last_request is enabling cfg80211 to
be built-in to the kernel, not enabling CFG80211_INTERNAL_REGDB
and the CRDA binary not being available at the time the udev
rule that kicks of CRDA triggers.

In such a situation we want to let some cfg80211 triggers
eventually kick CRDA for us again. Without this if the first
cycle attempt to kick off CRDA failed we'd be stuck without
the ability to change process any further regulatory domains.

cfg80211 will trigger re-processing of the regulatory queue
whenever schedule_work(&reg_work) is called, currently this
happens when:

  * suspend / resume
  * disconnect
  * a beacon hint gets triggered (non DFS 5 GHz AP found)
  * a regulatory request gets added to the queue

We don't have any specific opportunistic late boot triggers
to address a late mount of where CRDA resides though, adding
that should be done separately through another patch.
Without an opportunistic fix then this fix relies at least
one of the triggeres above to happen.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
 net/wireless/reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 081c571..625c41e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1912,7 +1912,7 @@ static void reg_process_pending_hints(void)
 
 	/* When last_request->processed becomes true this will be rescheduled */
 	if (lr && !lr->processed) {
-		REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
+		reg_process_hint(lr);
 		return;
 	}
 
-- 
1.9.0

Re: [PATCH 1/2] cfg80211: avoid freeing last_request while in flight

[Johannes Berg]

On Mon, 2014-04-21 at 20:39 -0700, Luis R. Rodriguez wrote:
> From: Arik Nemtsov <arik@wizery.com>
> 
> Avoid freeing the last request while it is being processed. This can
> happen in some cases if reg_work is kicked for some reason while the
> currently pending request is in flight.

Applied.

johannes

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Johannes Berg]

Applied, I hope it won't blow up again :)

johannes

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Sander Eikelenboom]

Tuesday, April 22, 2014, 5:18:19 PM, you wrote:

> Applied, I hope it won't blow up again :)

> johannes

At least there is some time left this time around ;-)
Will retest tomorrow to at least verify that it indeed also fixes something !

Thanks!

--
Sander

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Mon, 21 Apr 2014 20:39:33 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:



> that'd be appreciated as an alternative. I'm also very curious to hear
> no one else would have run into the same regression as you have.

I only see this on an Acer PTZ1825 with an ath9k wireless card, but
that machine was not sold with that wlan card, so other people with the
same model won't neccesarily see the same issue.


> From: "Luis R. Rodriguez" <mcgrof@suse.com>
> 
> This rebases Arik's patch to apply first and then piggy back
> the non modular fix on top of that. I think you all have
> more fancy things to test this series than I do now.
> 
> Michael Leun <lkml20140418@newton.leun.net> you reported a regression
> with commit 2a901468c221e778af52603e006a53 on v3.14, I however cannot
> see how that commit caused the issue you reported, can you test this
> patch ported onto v3.14? A port is here:
> 
> http://drvbp1.linux-foundation.org/~mcgrof/patches/2014/04/22/test-ariks-patch-for-314.patch

Applied this one ontop 3.14.1 and it does not seem to change anything:

When doing "rtcwake -m mem -s 30" I get 100% of tries the result below.

Did I get this correct, test-ariks-patch-for-314.patch is the same
thing as the two patches you posted and that got applied? If so, that
does not help for 3.14, I fear. I might try them on an current 3.15-rc.

[   97.914884] PM: Syncing filesystems ... done.
[   97.988935] PM: Preparing system for mem sleep
[   98.181751] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   98.183136] Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done.
[   98.184111] PM: Entering mem sleep   
[   98.184765] wlan0: deauthenticating from 90:f6:52:4e:ba:b6 by local choice (reason=3)
[   98.200666] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   98.201020] sd 0:0:0:0: [sda] Stopping disk
[   98.201150] BUG: unable to handle kernel NULL pointer dereference at 0000000000000038
[   98.201233] IP: [<ffffffffa002b1f1>] reg_todo+0xc1/0x4d0 [cfg80211]
[   98.201320] PGD 0
[   98.201348] Oops: 0000 [#1] SMP
[   98.201395] Modules linked in: netconsole configfs ipt_REJECT xt_LOG xt_limit xt_recent iptable_mangle iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 iptable_filter nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack xt_tcpudp ip6table_filter ip6_tables nf_nat_ftp
 nf_nat nf_conntrack_ftp nf_conntrack ip_tables x_tables ctr ccm snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_pcm acer_wmi iTCO_wdt iTCO_vendor_support sparse_keymap snd_seq snd_seq_device snd_timer 
snd_mixer_oss hid_multitouch coretemp snd pcspkr joydev ath9k atl1c serio_raw i2c_i801 ac shpchp lpc_ich mfd_core acpi_cpufreq battery soundcore wmi sg sha256_ssse3 sha256_generic cbc linear md_mod af_packet usbhid uhci_hcd i915 i2c_algo_bit drm_kms_helper drm ehci_pci e
hci_hcd video usbcore button usb_common scsi_dh_rdac scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw scsi_dh dm_mirror dm_region_hash dm_log dm_crypt dm_snapshot dm_bufio dm_mod glue_helper lrw gf128mul ablk_helper cryptd aes_x86_64 arc4 ath9k_common ath9k_hw ath mac80211 cfg8021
1 rfkill fan processor thermal [last unloaded: ath9k]
[   98.202874] CPU: 1 PID: 22 Comm: kworker/1:0 Tainted: G          I  3.14.1-test+ #2
[   98.202936] Hardware name: Acer Aspire 1825PTZ/JM12-MS-CAP, BIOS V1.3127 04/23/2010
[   98.203017] Workqueue: events reg_todo [cfg80211]
[   98.203064] task: ffff8801b71c2610 ti: ffff8801b71c4000 task.ti: ffff8801b71c4000
[   98.203124] RIP: 0010:[<ffffffffa002b1f1>]  [<ffffffffa002b1f1>] reg_todo+0xc1/0x4d0 [cfg80211]
[   98.203219] RSP: 0018:ffff8801b71c5de8  EFLAGS: 00010297
[   98.203265] RAX: ffffffffa0074360 RBX: ffff8801b1dfb2f0 RCX: 0000000000000003
[   98.203322] RDX: 0000000000000000 RSI: ffff8801bfc92ed8 RDI: 0000000000000000
[   98.203378] RBP: ffff8801b71c5e20 R08: fffe801d0b000000 R09: 1600000000000000
[   98.203435] R10: 00017fe2950742c0 R11: 0000000000000000 R12: ffff8801b1dfb2c0
[   98.203491] R13: 0000000000000000 R14: ffff8801b1dfb2dc R15: 0000000000000040
[   98.203549] FS:  0000000000000000(0000) GS:ffff8801bfc80000(0000) knlGS:0000000000000000
[   98.203615] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   98.203663] CR2: 0000000000000038 CR3: 0000000001a0e000 CR4: 00000000000407e0
[   98.203719] Stack:
[   98.203741]  ffff8801b71c5e20 ffffffffa00499df ffff8801b70e8300 ffff8801bfc92ec0
[   98.203829]  ffffffffa00742c0 0000000000000000 0000000000000040 ffff8801b71c5e68
[   98.203913]  ffffffff81071b48 00000000bfc92ed8 ffff8801bfc96a00 ffff8801bfc92ed8
[   98.203999] Call Trace:
[   98.204053]  [<ffffffffa00499df>] ? disconnect_work+0xbf/0xd0 [cfg80211]
[   98.204120]  [<ffffffff81071b48>] process_one_work+0x178/0x420
[   98.204171]  [<ffffffff81072759>] worker_thread+0x119/0x3a0
[   98.204220]  [<ffffffff81072640>] ? rescuer_thread+0x360/0x360
[   98.204271]  [<ffffffff81078ced>] kthread+0xcd/0xf0
[   98.204314]  [<ffffffff81078c20>] ? kthread_create_on_node+0x180/0x180
[   98.204371]  [<ffffffff8159300c>] ret_from_fork+0x7c/0xb0
[   98.204417]  [<ffffffff81078c20>] ? kthread_create_on_node+0x180/0x180
[   98.204470] Code: 8e 02 00 00 83 f9 01 0f 84 d7 01 00 00 72 4a 83 f9 03 0f 85 ad 01 00 00 48 8b 05 6b 91 04 00 8b 50 14 83 fa 01 0f 84 79 03 00 00 <41> f6 45 38 10 0f 85 49 03 00 00 0f b6 4b ec f6 81 c0 5c 64 81
[   98.205094] RIP  [<ffffffffa002b1f1>] reg_todo+0xc1/0x4d0 [cfg80211]
[   98.205108]  RSP <ffff8801b71c5de8>
[   98.205108] CR2: 0000000000000038
[   98.205108] ---[ end trace 5ba75e439c2c7126 ]---
[   98.213229] BUG: unable to handle kernel paging request at ffffffffffffffd8
[   98.215861] IP: [<ffffffff8107928b>] kthread_data+0xb/0x20
[   98.217042] PGD 1a0f067 PUD 1a11067 PMD 0
[   98.217042] Oops: 0000 [#2] SMP
[   98.217042] Modules linked in: netconsole configfs ipt_REJECT xt_LOG xt_limit xt_recent iptable_mangle iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 iptable_filter nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack xt_tcpudp ip6table_filter ip6_tables nf_nat_ftp nf_nat nf_conntrack_ftp nf_conntrack ip_tables x_tables ctr ccm snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hwdep snd_pcm_oss snd_pcm acer_wmi iTCO_wdt iTCO_vendor_support sparse_keymap snd_seq snd_seq_device snd_timer snd_mixer_oss hid_multitouch coretemp snd pcspkr joydev ath9k atl1c serio_raw i2c_i801 ac shpchp lpc_ich mfd_core acpi_cpufreq battery soundcore wmi sg sha256_ssse3 sha256_generic cbc linear md_mod af_packet usbhid uhci_hcd i915 i2c_algo_bit drm_kms_helper drm ehci_pci ehci_hcd video usbcore button usb_common scsi_dh_rdac scsi_dh_alua scsi_dh_emc scsi_dh_hp_sw scsi_dh dm_mirror dm_region_hash dm_log dm_crypt dm_snapshot dm_bufio dm_mod glue_he!
 lper lrw gf128mul ablk_helper cryptd aes_x86_64 arc4 ath9k_common ath9k_hw ath mac80211 cfg80211 rfkill fan processor thermal [last unloaded: ath9k]
[   98.217042] CPU: 1 PID: 22 Comm: kworker/1:0 Tainted: G      D   I  3.14.1-test+ #2
[   98.217042] Hardware name: Acer Aspire 1825PTZ/JM12-MS-CAP, BIOS V1.3127 04/23/2010
[   98.217042] task: ffff8801b71c2610 ti: ffff8801b71c4000 task.ti: ffff8801b71c4000
[   98.217042] RIP: 0010:[<ffffffff8107928b>]  [<ffffffff8107928b>] kthread_data+0xb/0x20
[   98.217042] RSP: 0018:ffff8801b71c5a40  EFLAGS: 00010002
[   98.217042] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000007
[   98.217042] RDX: 0000000000000002 RSI: 0000000000000001 RDI: ffff8801b71c2610
[   98.217042] RBP: ffff8801b71c5a40 R08: 0000000000000000 R09: 0000000000000001
[   98.217042] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8801b71c2b18
[   98.217042] R13: 0000000000000001 R14: ffff8801b71c2600 R15: ffff8801b71c2610
[   98.217042] FS:  0000000000000000(0000) GS:ffff8801bfc80000(0000) knlGS:0000000000000000
[   98.217042] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[   98.217042] CR2: 0000000000000028 CR3: 0000000001a0e000 CR4: 00000000000407e0
[   98.217042] Stack:
[   98.217042]  ffff8801b71c5a58 ffffffff81072d5c ffff8801bfc93640 ffff8801b71c5ab8
[   98.217042]  ffffffff8158753b ffff8801b71c2610 ffff8801b71c5fd8 0000000000013640
[   98.217042]  0000000000013640 ffff8801b71c2610 ffff8801b71c2d38 ffff8801b71c2600
[   98.217042] Call Trace:
[   98.217042]  [<ffffffff81072d5c>] wq_worker_sleeping+0xc/0x80
[   98.217042]  [<ffffffff8158753b>] __schedule+0x51b/0x750
[   98.217042]  [<ffffffff81587794>] schedule+0x24/0x70
[   98.217042]  [<ffffffff8105974f>] do_exit+0x6cf/0xa40
[   98.217042]  [<ffffffff810ad8c8>] ? console_unlock+0x1e8/0x3f0
[   98.217042]  [<ffffffff8158bd07>] oops_end+0x97/0xe0
[   98.217042]  [<ffffffff8157f22e>] no_context+0x279/0x286
[   98.217042]  [<ffffffff8157f2a9>] __bad_area_nosemaphore+0x6e/0x1c7
[   98.217042]  [<ffffffff8157f410>] bad_area_nosemaphore+0xe/0x10
[   98.217042]  [<ffffffff8158e26c>] __do_page_fault+0x8c/0x520
[   98.217042]  [<ffffffff8100a825>] ? native_sched_clock+0x35/0x90
[   98.217042]  [<ffffffff8100a889>] ? sched_clock+0x9/0x10
[   98.217042]  [<ffffffff8108bd3d>] ? sched_clock_local+0x1d/0x80
[   98.217042]  [<ffffffff8108bf68>] ? sched_clock_cpu+0x98/0xc0
[   98.217042]  [<ffffffff8106fa10>] ? insert_work+0x50/0xa0
[   98.217042]  [<ffffffff8158e722>] do_page_fault+0x22/0x30
[   98.217042]  [<ffffffff8158b1b8>] page_fault+0x28/0x30
[   98.217042]  [<ffffffffa002b1f1>] ? reg_todo+0xc1/0x4d0 [cfg80211]
[   98.217042]  [<ffffffffa002b1bb>] ? reg_todo+0x8b/0x4d0 [cfg80211]
[   98.217042]  [<ffffffffa00499df>] ? disconnect_work+0xbf/0xd0 [cfg80211]
[   98.217042]  [<ffffffff81071b48>] process_one_work+0x178/0x420
[   98.217042]  [<ffffffff81072759>] worker_thread+0x119/0x3a0
[   98.217042]  [<ffffffff81072640>] ? rescuer_thread+0x360/0x360
[   98.217042]  [<ffffffff81078ced>] kthread+0xcd/0xf0
[   98.217042]  [<ffffffff81078c20>] ? kthread_create_on_node+0x180/0x180
[   98.217042]  [<ffffffff8159300c>] ret_from_fork+0x7c/0xb0
[   98.217042]  [<ffffffff81078c20>] ? kthread_create_on_node+0x180/0x180
[   98.217042] Code: 00 48 89 e5 5d 48 8b 40 c8 48 c1 e8 02 83 e0 01 c3 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 48 8b 87 b0 04 00 00 55 48 89 e5 <48> 8b 40 d8 5d c3 66 66 66 66 66 66 2e 0f 1f 84 00 00 00 00 00
[   98.217042] RIP  [<ffffffff8107928b>] kthread_data+0xb/0x20
[   98.217042]  RSP <ffff8801b71c5a40>
[   98.217042] CR2: ffffffffffffffd8  
[   98.217042] ---[ end trace 5ba75e439c2c7127 ]---
[   98.217042] Fixing recursive fault but reboot is needed!
[   99.169250] Kernel panic - not syncing: Watchdog detected hard LOCKUP on cpu 0
[   99.169250] Shutting down cpus with NMI
[   99.169250] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff9fffffff)
[   99.169250] drm_kms_helper: panic occurred, switching back to text console
[   99.169250] Rebooting in 15 seconds..


> 
> Arik Nemtsov (1):
>   cfg80211: avoid freeing last_request while in flight
> 
> Luis R. Rodriguez (1):
>   cfg80211: fix processing world regdomain when non modular
> 
>  net/wireless/reg.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.0
> 


-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 9:00 AM, Michael Leun
<lkml20140418@newton.leun.net> wrote:
> Did I get this correct, test-ariks-patch-for-314.patch is the same
> thing as the two patches you posted and that got applied? If so, that
> does not help for 3.14, I fear. I might try them on an current 3.15-rc.

No its just one patch, the first one. I'll have another look at this,
do you happen to have gdb? Can you do:

gdb net/wireless/cfg80211.ko
l *reg_todo+0xc1

  Luis

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 8:37 AM, Sander Eikelenboom
<linux@eikelenboom.it> wrote:
>
> Tuesday, April 22, 2014, 5:18:19 PM, you wrote:
>
>> Applied, I hope it won't blow up again :)
>
>> johannes
>
> At least there is some time left this time around ;-)
> Will retest tomorrow to at least verify that it indeed also fixes something !

FWIW you had already tested the second patch which had solved your
issue, the first patch is an enhancement fix that addresses two
regressions introduced by enabling reprocessing of the last request
which as Arik found was caused by treating free'ing a new request (or
reprocessing after my patch) being processed as a last request. One
aspect of the full series I had originally sent out is still not
merged and that is the last patch which added opportunistic triggers
to check the regulatory queue after bootup. That was rejected based on
an architectural design to compartmentalize regulatory and while I
could argue with it, over time I think Johannes is right and will send
out a follow up patch with a timer for it eventually. The other
triggers for hitting the queue can be manual (userspace iw reg set) or
beacon hints on 5 GHz channels (most folks will have this) my goal was
to automate something to kick it after boot up.

  Luis

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Tue, 22 Apr 2014 09:32:03 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:

> On Tue, Apr 22, 2014 at 9:00 AM, Michael Leun
> <lkml20140418@newton.leun.net> wrote:
> > Did I get this correct, test-ariks-patch-for-314.patch is the same
> > thing as the two patches you posted and that got applied? If so,
> > that does not help for 3.14, I fear. I might try them on an current
> > 3.15-rc.
> 
> No its just one patch, the first one. I'll have another look at this,
> do you happen to have gdb? Can you do:
> 
> gdb net/wireless/cfg80211.ko
> l *reg_todo+0xc1

Do you prefer with or without test-ariks-patch-for-314.patch? This is
with, if you need anything else please let me know.

Note: I already had reversed the patch, I've put it back in and
recompiled - if that changes anything when correlating this with the
oops I posted...


ml@jill:/usr/src/kernel/linux1> gdb net/wireless/cfg80211.ko
GNU gdb (GDB; openSUSE 13.1) 7.6.50.20130731-cvs
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-suse-linux".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://bugs.opensuse.org/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word".
..
Reading symbols from /usr/src/kernel/linux1/net/wireless/cfg80211.ko...done.
(gdb) l *reg_todo+0xc1
0x61f1 is in reg_todo (net/wireless/reg.c:1611).
1606                    /* Trust a Cell base station over the AP's country IE */
1607                    if (regdom_changes(country_ie_request->alpha2))
1608                            return REG_REQ_IGNORE;
1609                    return REG_REQ_ALREADY_SET;
1610            } else {
1611                    if (wiphy->regulatory_flags & REGULATORY_COUNTRY_IE_IGNORE)
1612                            return REG_REQ_IGNORE;
1613            }
1614
1615            if (unlikely(!is_an_alpha2(country_ie_request->alpha2)))
(gdb) quit
ml@jill:/usr/src/kernel/linux1>


-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 9:48 AM, Michael Leun
<lkml20140418@newton.leun.net> wrote:
> 1611                    if (wiphy->regulatory_flags & REGULATORY_COUNTRY_IE_IGNORE)
> 1612                            return REG_REQ_IGNORE;

Interesting -- same line as your regression test had found, wiphy->
must be NULL. Digging deeper, you can discard that patch now, we can
rule out the reprocessing of the request.

 Luis

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Tue, 22 Apr 2014 10:04:19 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:

> On Tue, Apr 22, 2014 at 9:48 AM, Michael Leun
> <lkml20140418@newton.leun.net> wrote:
> > 1611                    if (wiphy->regulatory_flags &
> > REGULATORY_COUNTRY_IE_IGNORE) 1612
> > return REG_REQ_IGNORE;
> 
> Interesting -- same line as your regression test had found, wiphy->
> must be NULL. Digging deeper, you can discard that patch now, we can
> rule out the reprocessing of the request.


OK, but if this adds any useful information: Meanwhile I applied the
2nd patch you posted ontop test-ariks-patch-for-314.patch (with some
fuzz), and that did change something:

Does not wait until I try to hibernate to bang, it barfs already during
boot and that seems to look different. Didnt have a chance to copy that
yet, but could, if useful.

-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 10:30 AM, Michael Leun
<lkml20140418@newton.leun.net> wrote:
> On Tue, 22 Apr 2014 10:04:19 -0700
> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
>
>> On Tue, Apr 22, 2014 at 9:48 AM, Michael Leun
>> <lkml20140418@newton.leun.net> wrote:
>> > 1611                    if (wiphy->regulatory_flags &
>> > REGULATORY_COUNTRY_IE_IGNORE) 1612
>> > return REG_REQ_IGNORE;
>>
>> Interesting -- same line as your regression test had found, wiphy->
>> must be NULL. Digging deeper, you can discard that patch now, we can
>> rule out the reprocessing of the request.
>
>
> OK, but if this adds any useful information: Meanwhile I applied the
> 2nd patch you posted ontop test-ariks-patch-for-314.patch (with some
> fuzz), and that did change something:
>
> Does not wait until I try to hibernate to bang, it barfs already during
> boot and that seems to look different. Didnt have a chance to copy that
> yet, but could, if useful.

Yeah we don't want that 2nd patch, your issue seems like a race on the
 wiphy during suspend, which is very surprising, specially that no one
else would have picked up on this before. I'll go build v3.14.1 and
reproduce with rtcwake -m mem -s 30.

  Luis

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Tue, 22 Apr 2014 10:38:54 -0700
"Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:


> Yeah we don't want that 2nd patch, your issue seems like a race on the
>  wiphy during suspend, which is very surprising, specially that no one
> else would have picked up on this before. I'll go build v3.14.1 and
> reproduce with rtcwake -m mem -s 30.

Suspend is only one thing that reliably triggers this. I've also seen
the machine to barf during normal operation, but only since 3.14.

Hope you can reproduce, if not I maybe have to think about how to crank
up some remote access for you...

-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 06:00:03PM +0200, Michael Leun wrote:
> On Mon, 21 Apr 2014 20:39:33 -0700
> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
> 
> 
> 
> > that'd be appreciated as an alternative. I'm also very curious to hear
> > no one else would have run into the same regression as you have.
> 
> I only see this on an Acer PTZ1825 with an ath9k wireless card, but
> that machine was not sold with that wlan card, so other people with the
> same model won't neccesarily see the same issue.

OK I see the issue and its clear now how this could have
gone through, the wiphy comes from the country IE and it
could have gone disappearing during suspend / hotplug, so
we do need to check for it again.

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index f054137..4f375b6 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1602,7 +1602,8 @@ __reg_process_hint_country_ie(struct wiphy *wiphy,
 			return REG_REQ_IGNORE;
 		return REG_REQ_ALREADY_SET;
 	} else {
-		if (wiphy->regulatory_flags & REGULATORY_COUNTRY_IE_IGNORE)
+		if (wiphy &&
+		    wiphy->regulatory_flags & REGULATORY_COUNTRY_IE_IGNORE)
 			return REG_REQ_IGNORE;
 	}
 

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Tue, 22 Apr 2014 22:45:52 +0200
"Luis R. Rodriguez" <mcgrof@suse.com> wrote:


> > I only see this on an Acer PTZ1825 with an ath9k wireless card, but
> > that machine was not sold with that wlan card, so other people with
> > the same model won't neccesarily see the same issue.
> 
> OK I see the issue and its clear now how this could have
> gone through, the wiphy comes from the country IE and it
> could have gone disappearing during suspend / hotplug, so
> we do need to check for it again.
[...]
> -		if (wiphy->regulatory_flags &
> REGULATORY_COUNTRY_IE_IGNORE)
> +		if (wiphy &&
> +		    wiphy->regulatory_flags &
> REGULATORY_COUNTRY_IE_IGNORE) return REG_REQ_IGNORE;

Yup, thats it. I can hear it whisper "put me into stable".

Thanks!

-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Tue, Apr 22, 2014 at 11:32:13PM +0200, Michael Leun wrote:
> On Tue, 22 Apr 2014 22:45:52 +0200
> "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
> 
> 
> > > I only see this on an Acer PTZ1825 with an ath9k wireless card, but
> > > that machine was not sold with that wlan card, so other people with
> > > the same model won't neccesarily see the same issue.
> > 
> > OK I see the issue and its clear now how this could have
> > gone through, the wiphy comes from the country IE and it
> > could have gone disappearing during suspend / hotplug, so
> > we do need to check for it again.
> [...]
> > -		if (wiphy->regulatory_flags &
> > REGULATORY_COUNTRY_IE_IGNORE)
> > +		if (wiphy &&
> > +		    wiphy->regulatory_flags &
> > REGULATORY_COUNTRY_IE_IGNORE) return REG_REQ_IGNORE;
> 
> Yup, thats it. I can hear it whisper "put me into stable".

I was a bit perplexed that this was not picked up before, so I hunted
a bit and found a fix that was applied to the mac80211-next upstream
development tree but not propagated to stable. Can you remove that pach,
just git checkout -f and try this one instead? It applies cleanly and
since its upstream on Linus' tree already we should be able to get this
to Greg faster if you confirm it fixes your issue. So all we need is
a Tested-by.

  Luis

commit 772f0389338cfcf96da1c178046dc7e1649ab554
Author: Ilan Peer <ilan.peer@intel.com>
Date:   Tue Jan 14 15:17:23 2014 +0200

    cfg80211: fix few minor issues in reg_process_hint()
    
    Fix the following issues in reg_process_hint():
    
    1. Add verification that wiphy is valid before processing
       NL80211_REGDOMAIN_SET_BY_COUNTRY_IE.
    2. Free the request in case of invalid initiator.
    3. Remove WARN_ON check on reg_request->alpha2 as it is not a
       pointer.
    
    Signed-off-by: Ilan Peer <ilan.peer@intel.com>
    Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 9b897fc..484facf 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1683,17 +1683,9 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 	struct wiphy *wiphy = NULL;
 	enum reg_request_treatment treatment;
 
-	if (WARN_ON(!reg_request->alpha2))
-		return;
-
 	if (reg_request->wiphy_idx != WIPHY_IDX_INVALID)
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
 
-	if (reg_request->initiator == NL80211_REGDOM_SET_BY_DRIVER && !wiphy) {
-		kfree(reg_request);
-		return;
-	}
-
 	switch (reg_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
 		reg_process_hint_core(reg_request);
@@ -1706,20 +1698,29 @@ static void reg_process_hint(struct regulatory_request *reg_request)
 		schedule_delayed_work(&reg_timeout, msecs_to_jiffies(3142));
 		return;
 	case NL80211_REGDOM_SET_BY_DRIVER:
+		if (!wiphy)
+			goto out_free;
 		treatment = reg_process_hint_driver(wiphy, reg_request);
 		break;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
+		if (!wiphy)
+			goto out_free;
 		treatment = reg_process_hint_country_ie(wiphy, reg_request);
 		break;
 	default:
 		WARN(1, "invalid initiator %d\n", reg_request->initiator);
-		return;
+		goto out_free;
 	}
 
 	/* This is required so that the orig_* parameters are saved */
 	if (treatment == REG_REQ_ALREADY_SET && wiphy &&
 	    wiphy->regulatory_flags & REGULATORY_STRICT_REG)
 		wiphy_update_regulatory(wiphy, reg_request->initiator);
+
+	return;
+
+out_free:
+	kfree(reg_request);
 }
 
 /*

Re: [PATCH 0/2] cfg80211: two reg fixes

[Michael Leun]

On Tue, 22 Apr 2014 23:50:12 +0200
"Luis R. Rodriguez" <mcgrof@suse.com> wrote:

> On Tue, Apr 22, 2014 at 11:32:13PM +0200, Michael Leun wrote:
> > On Tue, 22 Apr 2014 22:45:52 +0200
> > "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
> > 
> > 
> > > > I only see this on an Acer PTZ1825 with an ath9k wireless card,
> > > > but that machine was not sold with that wlan card, so other
> > > > people with the same model won't neccesarily see the same issue.
> > > 
> > > OK I see the issue and its clear now how this could have
> > > gone through, the wiphy comes from the country IE and it
> > > could have gone disappearing during suspend / hotplug, so
> > > we do need to check for it again.
> > [...]
> > > -		if (wiphy->regulatory_flags &
> > > REGULATORY_COUNTRY_IE_IGNORE)
> > > +		if (wiphy &&
> > > +		    wiphy->regulatory_flags &
> > > REGULATORY_COUNTRY_IE_IGNORE) return REG_REQ_IGNORE;
> > 
> > Yup, thats it. I can hear it whisper "put me into stable".
> 
> I was a bit perplexed that this was not picked up before, so I hunted
> a bit and found a fix that was applied to the mac80211-next upstream
> development tree but not propagated to stable. Can you remove that
> pach, just git checkout -f and try this one instead? It applies
> cleanly and since its upstream on Linus' tree already we should be
> able to get this to Greg faster if you confirm it fixes your issue.
> So all we need is a Tested-by.
> 
>   Luis
> 
> commit 772f0389338cfcf96da1c178046dc7e1649ab554
> Author: Ilan Peer <ilan.peer@intel.com>


Yes, this also fixes it.

-- 
MfG,

Michael Leun

Re: [PATCH 0/2] cfg80211: two reg fixes

[Luis R. Rodriguez]

On Wed, Apr 23, 2014 at 12:53:09AM +0200, Michael Leun wrote:
> On Tue, 22 Apr 2014 23:50:12 +0200
> "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
> 
> > On Tue, Apr 22, 2014 at 11:32:13PM +0200, Michael Leun wrote:
> > > On Tue, 22 Apr 2014 22:45:52 +0200
> > > "Luis R. Rodriguez" <mcgrof@suse.com> wrote:
> > > 
> > > 
> > > > > I only see this on an Acer PTZ1825 with an ath9k wireless card,
> > > > > but that machine was not sold with that wlan card, so other
> > > > > people with the same model won't neccesarily see the same issue.
> > > > 
> > > > OK I see the issue and its clear now how this could have
> > > > gone through, the wiphy comes from the country IE and it
> > > > could have gone disappearing during suspend / hotplug, so
> > > > we do need to check for it again.
> > > [...]
> > > > -		if (wiphy->regulatory_flags &
> > > > REGULATORY_COUNTRY_IE_IGNORE)
> > > > +		if (wiphy &&
> > > > +		    wiphy->regulatory_flags &
> > > > REGULATORY_COUNTRY_IE_IGNORE) return REG_REQ_IGNORE;
> > > 
> > > Yup, thats it. I can hear it whisper "put me into stable".
> > 
> > I was a bit perplexed that this was not picked up before, so I hunted
> > a bit and found a fix that was applied to the mac80211-next upstream
> > development tree but not propagated to stable. Can you remove that
> > pach, just git checkout -f and try this one instead? It applies
> > cleanly and since its upstream on Linus' tree already we should be
> > able to get this to Greg faster if you confirm it fixes your issue.
> > So all we need is a Tested-by.
> > 
> >   Luis
> > 
> > commit 772f0389338cfcf96da1c178046dc7e1649ab554
> > Author: Ilan Peer <ilan.peer@intel.com>
> 
> 
> Yes, this also fixes it.

OK thanks, will poke stable, and thanks for the report.

  Luis

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Helmut Schaa]

Hi Luis,

On Tue, Apr 22, 2014 at 5:39 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> This allows processing of the last regulatory request when
> we determine its still pending. Without this if a regulatory
> request failed to get processed by userspace we wouldn't
> be able to re-process it later. An example situation that can
> lead to an unprocessed last_request is enabling cfg80211 to
> be built-in to the kernel, not enabling CFG80211_INTERNAL_REGDB
> and the CRDA binary not being available at the time the udev
> rule that kicks of CRDA triggers.
>
> In such a situation we want to let some cfg80211 triggers
> eventually kick CRDA for us again. Without this if the first
> cycle attempt to kick off CRDA failed we'd be stuck without
> the ability to change process any further regulatory domains.
>
> cfg80211 will trigger re-processing of the regulatory queue
> whenever schedule_work(&reg_work) is called, currently this
> happens when:
>
>   * suspend / resume
>   * disconnect
>   * a beacon hint gets triggered (non DFS 5 GHz AP found)
>   * a regulatory request gets added to the queue
>
> We don't have any specific opportunistic late boot triggers
> to address a late mount of where CRDA resides though, adding
> that should be done separately through another patch.
> Without an opportunistic fix then this fix relies at least
> one of the triggeres above to happen.
>
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
>  net/wireless/reg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 081c571..625c41e 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1912,7 +1912,7 @@ static void reg_process_pending_hints(void)
>
>         /* When last_request->processed becomes true this will be rescheduled */
>         if (lr && !lr->processed) {
> -               REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
> +               reg_process_hint(lr);
>                 return;
>         }
>

This change created a race in my setup (ar9344 based AP with two ath9k devices):
Sometimes during boot the following happens:

...
[    6.190000] ath: Country alpha2 being used: US
...
[    6.210000] ath: Country alpha2 being used: US
...
[    6.240000] cfg80211: Calling CRDA for country: US
[    6.240000] cfg80211: Calling CRDA for country: US
[    6.240000] cfg80211: Current regulatory domain intersected:
[    6.240000] cfg80211:  DFS Master region: unset
[    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp), (dfs_cac_time)
[    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
(N/A, 1700 mBm), (N/A)
[    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (0 s)
[    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
KHz), (N/A, 0 mBm), (N/A)
...

CRDA (or in my case the internal regdb) is called twice for country
code US by two different devices.
In this case the second request came in before the first one was
completely processed and the first
request will be re-processed (due to this patch) leading to an
intersected regdomain (US with US :) ).

Reverting this patch fixes this race for me but I think you had some
good reasons to do this patch.

Any ideas?

Thanks,
Helmut

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

On Tue, Sep 02, 2014 at 02:00:00PM +0200, Helmut Schaa wrote:
> Hi Luis,
> 
> On Tue, Apr 22, 2014 at 5:39 AM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This allows processing of the last regulatory request when
> > we determine its still pending. Without this if a regulatory
> > request failed to get processed by userspace we wouldn't
> > be able to re-process it later. An example situation that can
> > lead to an unprocessed last_request is enabling cfg80211 to
> > be built-in to the kernel, not enabling CFG80211_INTERNAL_REGDB
> > and the CRDA binary not being available at the time the udev
> > rule that kicks of CRDA triggers.
> >
> > In such a situation we want to let some cfg80211 triggers
> > eventually kick CRDA for us again. Without this if the first
> > cycle attempt to kick off CRDA failed we'd be stuck without
> > the ability to change process any further regulatory domains.
> >
> > cfg80211 will trigger re-processing of the regulatory queue
> > whenever schedule_work(&reg_work) is called, currently this
> > happens when:
> >
> >   * suspend / resume
> >   * disconnect
> >   * a beacon hint gets triggered (non DFS 5 GHz AP found)
> >   * a regulatory request gets added to the queue
> >
> > We don't have any specific opportunistic late boot triggers
> > to address a late mount of where CRDA resides though, adding
> > that should be done separately through another patch.
> > Without an opportunistic fix then this fix relies at least
> > one of the triggeres above to happen.
> >
> > Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> > ---
> >  net/wireless/reg.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > index 081c571..625c41e 100644
> > --- a/net/wireless/reg.c
> > +++ b/net/wireless/reg.c
> > @@ -1912,7 +1912,7 @@ static void reg_process_pending_hints(void)
> >
> >         /* When last_request->processed becomes true this will be rescheduled */
> >         if (lr && !lr->processed) {
> > -               REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
> > +               reg_process_hint(lr);
> >                 return;
> >         }
> >
> 
> This change created a race in my setup (ar9344 based AP with two ath9k devices):
> Sometimes during boot the following happens:
> 
> ...
> [    6.190000] ath: Country alpha2 being used: US
> ...
> [    6.210000] ath: Country alpha2 being used: US
> ...
> [    6.240000] cfg80211: Calling CRDA for country: US
> [    6.240000] cfg80211: Calling CRDA for country: US
> [    6.240000] cfg80211: Current regulatory domain intersected:
> [    6.240000] cfg80211:  DFS Master region: unset
> [    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
> (max_antenna_gain, max_eirp), (dfs_cac_time)
> [    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
> (N/A, 1700 mBm), (N/A)
> [    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (0 s)
> [    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
> KHz), (N/A, 0 mBm), (N/A)
> ...
> 
> CRDA (or in my case the internal regdb) is called twice for country
> code US by two different devices.
> In this case the second request came in before the first one was
> completely processed and the first
> request will be re-processed (due to this patch) leading to an
> intersected regdomain (US with US :) ).

Doh yeah that's an issue, thanks for reporting this.

> Reverting this patch fixes this race for me but I think you had some
> good reasons to do this patch.

Indeed.

> Any ideas?

Yeah this seems to be a corner case of the fact that we deal with
locking for the last request only through RCU and we only annotate
that the request was processed but don't add checks for when its
about to be processed. At least that's what I gather could be
the issue here given that if you see __reg_process_hint_driver()
we already check for two driver hints matching and and just
request the core to copy the existing regulatory domain onto
the new device.

        if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
            !regdom_changes(driver_request->alpha2))
                return REG_REQ_ALREADY_SET;

        return REG_REQ_INTERSECT;

The intersection would happen otherwise and I think this is caused
by a small race here. Can you try the following:

diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 2599924..f3011d8 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -67,6 +67,10 @@ enum environment_cap {
  *	CRDA and can be used by other regulatory requests. When a
  *	the last request is not yet processed we must yield until it
  *	is processed before processing any new requests.
+ * @processing: tells the regulatory core if the request is currently
+ *	being processed. This is used to avoid contention on processing
+ *	a request twice since we treat only the last request atomically
+ *	with RCU.
  * @country_ie_checksum: checksum of the last processed and accepted
  *	country IE
  * @country_ie_env: lets us know if the AP is telling us we are outdoor,
@@ -82,6 +86,7 @@ struct regulatory_request {
 	enum nl80211_dfs_regions dfs_region;
 	bool intersect;
 	bool processed;
+	bool processing;
 	enum environment_cap country_ie_env;
 	struct list_head list;
 };
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 1afdf45..ae35385 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1912,7 +1912,7 @@ static void reg_process_pending_hints(void)
 	lr = get_last_request();
 
 	/* When last_request->processed becomes true this will be rescheduled */
-	if (lr && !lr->processed) {
+	if (lr && !lr->processed && !lr->processing) {
 		reg_process_hint(lr);
 		return;
 	}
@@ -2595,6 +2595,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	lr = get_last_request();
+	lr->processing = true;
 
 	/* Note that this doesn't update the wiphys, this is done below */
 	switch (lr->initiator) {

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Helmut Schaa]

Hi,

On Fri, Sep 5, 2014 at 11:43 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> Yeah this seems to be a corner case of the fact that we deal with
> locking for the last request only through RCU and we only annotate
> that the request was processed but don't add checks for when its
> about to be processed. At least that's what I gather could be
> the issue here given that if you see __reg_process_hint_driver()
> we already check for two driver hints matching and and just
> request the core to copy the existing regulatory domain onto
> the new device.
>
>         if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
>             !regdom_changes(driver_request->alpha2))
>                 return REG_REQ_ALREADY_SET;
>
>         return REG_REQ_INTERSECT;
>
> The intersection would happen otherwise and I think this is caused
> by a small race here. Can you try the following:

Just retried with your patch applied but I can still get an "98"
intersected regdomain :(

[    6.190000] ath: EEPROM regdomain: 0x0
[    6.190000] ath: EEPROM indicates default country code should be
used
[    6.190000] ath: doing EEPROM country->regdmn map search
[    6.190000] ath: country maps to regdmn code: 0x3a
[    6.190000] ath: Country alpha2 being used: US
[    6.190000] ath: Regpair used: 0x3a
[    6.200000] ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
[    6.210000] ieee80211 phy0: Atheros AR9340 Rev:0 mem=0xb8100000, irq=47
[    6.210000] PCI: Enabling device 0000:00:00.0 (0000 -> 0002)
[    6.210000] ath: EEPROM regdomain: 0x0
[    6.210000] ath: EEPROM indicates default country code should be used
[    6.210000] ath: doing EEPROM country->regdmn map search
[    6.210000] ath: country maps to regdmn code: 0x3a
[    6.210000] ath: Country alpha2 being used: US
[    6.210000] ath: Regpair used: 0x3a
[    6.220000] ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'
[    6.240000] cfg80211: Calling CRDA for country: US
[    6.240000] cfg80211: Calling CRDA for country: US
[    6.240000] cfg80211: Current regulatory domain intersected:
[    6.240000] cfg80211:  DFS Master region: unset
[    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp), (dfs_cac_time)
[    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
(N/A, 1700 mBm), (N/A)
[    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (0 s)
[    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
KHz), (N/A, 0 mBm), (N/A)
[    6.240000] cfg80211: Calling CRDA for country: US
[    6.240000] ieee80211 phy1: Atheros AR9300 Rev:3 mem=0xb0000000, irq=40
[    6.240000] cfg80211: Current regulatory domain intersected:
[    6.240000] cfg80211:  DFS Master region: unset
[    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
(max_antenna_gain, max_eirp), (dfs_cac_time)
[    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
(N/A, 1700 mBm), (N/A)
[    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (0 s)
[    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
(N/A, 2000 mBm), (N/A)
[    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
KHz), (N/A, 0 mBm), (N/A)

I'll try to dive deeper into the regulatory code ...
Helmut

Re: [PATCH 2/2] cfg80211: fix processing world regdomain when non modular

[Luis R. Rodriguez]

On Mon, Sep 08, 2014 at 11:59:39AM +0200, Helmut Schaa wrote:
> Hi,
> 
> On Fri, Sep 5, 2014 at 11:43 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > Yeah this seems to be a corner case of the fact that we deal with
> > locking for the last request only through RCU and we only annotate
> > that the request was processed but don't add checks for when its
> > about to be processed. At least that's what I gather could be
> > the issue here given that if you see __reg_process_hint_driver()
> > we already check for two driver hints matching and and just
> > request the core to copy the existing regulatory domain onto
> > the new device.
> >
> >         if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
> >             !regdom_changes(driver_request->alpha2))
> >                 return REG_REQ_ALREADY_SET;
> >
> >         return REG_REQ_INTERSECT;
> >
> > The intersection would happen otherwise and I think this is caused
> > by a small race here. Can you try the following:
> 
> Just retried with your patch applied but I can still get an "98"
> intersected regdomain :(
> 
> [    6.190000] ath: EEPROM regdomain: 0x0
> [    6.190000] ath: EEPROM indicates default country code should be
> used
> [    6.190000] ath: doing EEPROM country->regdmn map search
> [    6.190000] ath: country maps to regdmn code: 0x3a
> [    6.190000] ath: Country alpha2 being used: US
> [    6.190000] ath: Regpair used: 0x3a
> [    6.200000] ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
> [    6.210000] ieee80211 phy0: Atheros AR9340 Rev:0 mem=0xb8100000, irq=47
> [    6.210000] PCI: Enabling device 0000:00:00.0 (0000 -> 0002)
> [    6.210000] ath: EEPROM regdomain: 0x0
> [    6.210000] ath: EEPROM indicates default country code should be used
> [    6.210000] ath: doing EEPROM country->regdmn map search
> [    6.210000] ath: country maps to regdmn code: 0x3a
> [    6.210000] ath: Country alpha2 being used: US
> [    6.210000] ath: Regpair used: 0x3a
> [    6.220000] ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'
> [    6.240000] cfg80211: Calling CRDA for country: US
> [    6.240000] cfg80211: Calling CRDA for country: US
> [    6.240000] cfg80211: Current regulatory domain intersected:
> [    6.240000] cfg80211:  DFS Master region: unset
> [    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
> (max_antenna_gain, max_eirp), (dfs_cac_time)
> [    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
> (N/A, 1700 mBm), (N/A)
> [    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (0 s)
> [    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
> KHz), (N/A, 0 mBm), (N/A)
> [    6.240000] cfg80211: Calling CRDA for country: US
> [    6.240000] ieee80211 phy1: Atheros AR9300 Rev:3 mem=0xb0000000, irq=40
> [    6.240000] cfg80211: Current regulatory domain intersected:
> [    6.240000] cfg80211:  DFS Master region: unset
> [    6.240000] cfg80211:   (start_freq - end_freq @ bandwidth),
> (max_antenna_gain, max_eirp), (dfs_cac_time)
> [    6.240000] cfg80211:   (2402000 KHz - 2472000 KHz @ 40000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (2457000 KHz - 2472000 KHz @ 15000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (5170000 KHz - 5250000 KHz @ 80000 KHz),
> (N/A, 1700 mBm), (N/A)
> [    6.240000] cfg80211:   (5250000 KHz - 5330000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (0 s)
> [    6.240000] cfg80211:   (5735000 KHz - 5835000 KHz @ 80000 KHz),
> (N/A, 2000 mBm), (N/A)
> [    6.240000] cfg80211:   (57240000 KHz - 63720000 KHz @ 2160000
> KHz), (N/A, 0 mBm), (N/A)
> 
> I'll try to dive deeper into the regulatory code ...

OK thanks I appreciate that, it will take me a bit to get back to this
as this week and the next few weeks will be really busy and I will
be traveling.

  Luis