Re: [PATCH] cfg80211: Fix regulatory bug with multiple cards with a delayed CRDA response

[Mark Mentovai <mark@moxienet.com>]

Luis R. Rodriguez wrote:
> When two cards are connected with the same regulatory domain
> if CRDA had a delayed response then cfg80211's own set regulatory
> domain would still be the world regulatory domain. There was a bug
> on cfg80211's ignore_request() when analyzing incoming driver
> regulatory hints as it was only checking against the currently
> set cfg80211 regulatory domain and not for any other new
> pending requests. This is easily fixed by also checking against
> the pending request.

Luis, thanks for taking a look at this bug.

Capsule summary: you’ve overloaded -EALREADY, which until now seemed
to mean “that’s the regdomain I’m already using,” to now also mean
“I’m going to be using that regdomain soon but I’m waiting on CRDA.”
The two cases need to be handled differently.

In my case, this patch makes things a little bit worse. I tested it
with compat_wireless 20101110. I’ve included what I see after boot
below the explanation.

Your patch changes things so that, according to the steps in my
original message, steps 3 and 4 become:

3. The second card’s driver calls regulatory_hint to provide US as a
driver hint. ignore_request sees that the last request came from a
driver and that the current and last request sought to set the same
hint, so it returns -EALREADY. This triggers the “if the regulatory
domain being requested by the driver” block, which calls reg_copy_regd
on the apparent assumption that cfg80211_regdomain contains the
regdomain that the wiphy actually wants, although it doesn’t, and it’s
very wrong to do this copy. cfg80211_regdomain is still on 00, because
CRDA hasn’t responded to the first card’s request yet.

4. When CRDA finally responds to the first card’s request from #2, it
gets plumbed to set_regdom, which calls __set_regdom. __set_regdom
sees that it’s not intersecting, and enters the block where it would
normally copy reg_copy_regd to set the wiphy’s regd, but the wiphy it
uses is the one saved from last_request (in step 3), and it already
had something copied to it (also in step 3). Since __set_regdom checks
for this and bails out with -EALREADY in the “userspace could have
sent two replies with only one kernel request” case. Because
__set_regdom fails, set_regdom returns early and never makes it to the
update_all_wiphy_regulatory or print_regdomain steps. The regdomain
remains unchanged. I’m still stuck at 00.

What about using something other than -EALREADY to signal “that
request is already pending?” On its face, this works, but I think
there’s a deeper flaw that also needs to be addressed. I’m concerned
that the wiphys that fall into this state won’t see a reg_copy_regd
call at all. The idea is that any such wiphy shouldn’t really be
ignored, but it should be joined to a group of wiphys waiting on the
pending request, and when the request is satisfied, the regd field
should be populated for each of them.

With your patch:

# iw reg get
country 00:
        (2402 - 2472 @ 40), (3, 20)
        (2457 - 2482 @ 20), (3, 20), PASSIVE-SCAN, NO-IBSS
        (2474 - 2494 @ 20), (3, 20), NO-OFDM, PASSIVE-SCAN, NO-IBSS
        (5170 - 5250 @ 40), (3, 20), PASSIVE-SCAN, NO-IBSS
        (5735 - 5835 @ 40), (3, 20), PASSIVE-SCAN, NO-IBSS
# iw list (relevant bits)
Wiphy phy1
        Band 1:
                Frequencies:
                        * 5180 MHz [36] (17.0 dBm) (passive scanning, no IBSS)
                        * 5200 MHz [40] (17.0 dBm) (passive scanning, no IBSS)
                        * 5220 MHz [44] (17.0 dBm) (passive scanning, no IBSS)
                        * 5240 MHz [48] (17.0 dBm) (passive scanning, no IBSS)
                        * 5260 MHz [52] (disabled)
                        * 5280 MHz [56] (disabled)
                        * 5300 MHz [60] (disabled)
                        * 5320 MHz [64] (disabled)
                        * 5500 MHz [100] (disabled)
                        * 5520 MHz [104] (disabled)
                        * 5540 MHz [108] (disabled)
                        * 5560 MHz [112] (disabled)
                        * 5580 MHz [116] (disabled)
                        * 5600 MHz [120] (disabled)
                        * 5620 MHz [124] (disabled)
                        * 5640 MHz [128] (disabled)
                        * 5660 MHz [132] (disabled)
                        * 5680 MHz [136] (disabled)
                        * 5700 MHz [140] (disabled)
                        * 5745 MHz [149] (20.0 dBm) (passive scanning, no IBSS)
                        * 5765 MHz [153] (20.0 dBm) (passive scanning, no IBSS)
                        * 5785 MHz [157] (20.0 dBm) (passive scanning, no IBSS)
                        * 5805 MHz [161] (20.0 dBm) (passive scanning, no IBSS)
                        * 5825 MHz [165] (20.0 dBm) (passive scanning, no IBSS)
Wiphy phy0
        Band 1:
                Frequencies:
                        * 2412 MHz [1] (20.0 dBm)
                        * 2417 MHz [2] (20.0 dBm)
                        * 2422 MHz [3] (20.0 dBm)
                        * 2427 MHz [4] (20.0 dBm)
                        * 2432 MHz [5] (20.0 dBm)
                        * 2437 MHz [6] (20.0 dBm)
                        * 2442 MHz [7] (20.0 dBm)
                        * 2447 MHz [8] (20.0 dBm)
                        * 2452 MHz [9] (20.0 dBm)
                        * 2457 MHz [10] (20.0 dBm)
                        * 2462 MHz [11] (20.0 dBm)
                        * 2467 MHz [12] (20.0 dBm) (passive scanning, no IBSS)
                        * 2472 MHz [13] (20.0 dBm) (passive scanning, no IBSS)
                        * 2484 MHz [14] (20.0 dBm) (passive scanning, no IBSS)
# dmesg (relevant bits)
cfg80211: World regulatory domain updated:
    (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp)
    (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
    (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
    (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
    (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
    (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
PCI: Enabling device 0000:00:11.0 (0000 -> 0002)
ath: EEPROM regdomain: 0x0
ath: EEPROM indicates default country code should be used
ath: doing EEPROM country->regdmn map search
ath: country maps to regdmn code: 0x3a
ath: Country alpha2 being used: US
ath: Regpair used: 0x3a
ieee80211 phy0: Selected rate control algorithm 'minstrel_ht'
Registered led device: ath9k-phy0::radio
Registered led device: ath9k-phy0::assoc
Registered led device: ath9k-phy0::tx
Registered led device: ath9k-phy0::rx
ieee80211 phy0: Atheros AR9280 Rev:2 mem=0xb0000000, irq=48
PCI: Enabling device 0000:00:12.0 (0000 -> 0002)
ath: EEPROM regdomain: 0x0
ath: EEPROM indicates default country code should be used
ath: doing EEPROM country->regdmn map search
ath: country maps to regdmn code: 0x3a
ath: Country alpha2 being used: US
ath: Regpair used: 0x3a
cfg80211: Calling CRDA for country: US
ieee80211 phy1: Selected rate control algorithm 'minstrel_ht'
Registered led device: ath9k-phy1::radio
Registered led device: ath9k-phy1::assoc
Registered led device: ath9k-phy1::tx
Registered led device: ath9k-phy1::rx
ieee80211 phy1: Atheros AR9280 Rev:2 mem=0xb0010000, irq=49
(Nothing else from cfg80211, although I was hoping for “cfg80211:
Regulatory domain changed to country: US” and the new frequency
table.)

Incidentally, the workaround I’ve been using for the past few days
targeted the very same line your patch touches, although I used 0
instead of a special error flag. This resulted in an unnecessary extra
call out to CRDA, but more importantly, it had the “doesn’t set the
wiphy’s regd” problem, which is why I didn’t mention it.