Re: assert_cfg80211_lock warning with Linux v3.5-8833-g2d53492

["Arend van Spriel" <arend@broadcom.com>]

+ Luis

On 08/01/2012 05:53 PM, Johannes Berg wrote:
> On Wed, 2012-08-01 at 17:51 +0200, Arend van Spriel wrote:
>> On 08/01/2012 05:38 PM, Arend van Spriel wrote:
>>>> brcmsmac needs to hold cfg80211_mutex before calling freq_reg_info...
>>>>>
>>>>> It looks like those calls were added in mid-June.
>>>>>
>>> I think mid-june sounds about right. We never observed the warning when
>>> changes to use regulatory infrastructure were tested/reviewed. Should
>>> this precondition be mentioned in cfg80211.h?
>>>
>>> Gr. AvS
>>
>> Diving in further it seems brcmsmac can not grab the cfg80211_mutex. So
>> another solution is needed.
> 
> Yeah I was going to say -- how can it possibly access that? It seems
> that in some patch the API got broken, it should be taking the lock or
> whatever ... I'll leave it to Luis to sort out though :-P
> 
> johannes
> 

The assert was added by following commit:

commit ac46d48e00349c63650b3cc6f9460fcc183da6a6
Author: Luis R. Rodriguez <lrodriguez@atheros.com>
Date:   Fri May 1 18:44:50 2009 -0400

    cfg80211: fix race condition with wiphy_apply_custom_regulatory()

    We forgot to lock using the cfg80211_mutex in
    wiphy_apply_custom_regulatory(). Without the lock
    there is possible race between processing a reply from CRDA
    and a driver calling wiphy_apply_custom_regulatory(). During
    the processing of the reply from CRDA we free last_request and
    wiphy_apply_custom_regulatory() eventually accesses an
    element from last_request in the through freq_reg_info_regd().

    This is very difficult to reproduce (I haven't), it takes us
    3 hours and you need to be banging hard, but the race is obvious
    by looking at the code.

    This should only affect those who use this caller, which currently
    is ath5k, ath9k, and ar9170.

    EIP: 0060:[<f8ebec50>] EFLAGS: 00210282 CPU: 1
    EIP is at freq_reg_info_regd+0x24/0x121 [cfg80211]

It seems the API was as it currently is when adding regulatory framework
changes in brcmsmac so we should have seen this assert flying by. The
problem is that freq_reg_info() is exposed in cfg80211.h, but as it is
now it can only be used under the cfg80211_mutex lock, ie. in regulatory
notify callback (as Seth indicated).

Gr. AvS