[PATCH] mac80211: add missing RCU lock/unlock on ieee80211_rx_bss_info()

[PATCH] mac80211: add missing RCU lock/unlock on ieee80211_rx_bss_info()

[Luis R. Rodriguez]

Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 net/mac80211/ibss.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 0b30277..446cada 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -365,7 +365,9 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 		       sdata->dev->name, mgmt->bssid);
 #endif
 		ieee80211_sta_join_ibss(sdata, bss);
+		rcu_read_lock();
 		ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
+		rcu_read_unlock();
 	}
 
  put_bss:
-- 
1.6.0.6

Re: [PATCH] mac80211: add missing RCU lock/unlock on ieee80211_rx_bss_info()

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Tue, 2009-06-02 at 16:28 -0400, Luis R. Rodriguez wrote:
> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
> ---
>  net/mac80211/ibss.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 0b30277..446cada 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -365,7 +365,9 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
>  		       sdata->dev->name, mgmt->bssid);
>  #endif
>  		ieee80211_sta_join_ibss(sdata, bss);
> +		rcu_read_lock();
>  		ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
> +		rcu_read_unlock();
>  	}

I see no reason for this to be necessary.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH] mac80211: add missing RCU lock/unlock on ieee80211_rx_bss_info()

[Luis R. Rodriguez]

On Tue, Jun 2, 2009 at 1:34 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2009-06-02 at 16:28 -0400, Luis R. Rodriguez wrote:
>> Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
>> ---
>>  net/mac80211/ibss.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
>> index 0b30277..446cada 100644
>> --- a/net/mac80211/ibss.c
>> +++ b/net/mac80211/ibss.c
>> @@ -365,7 +365,9 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
>>                      sdata->dev->name, mgmt->bssid);
>>  #endif
>>               ieee80211_sta_join_ibss(sdata, bss);
>> +             rcu_read_lock();
>>               ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
>> +             rcu_read_unlock();
>>       }
>
> I see no reason for this to be necessary.

Hm yeah, I failed to see it doesn't get the sta before allocating it
and/or adding it. Is that right for this particular case? If the sta
does exist we wouldn't have set up the things for it that
ieee80211_ibss_add_sta() is setting.

  Luis

Re: [PATCH] mac80211: add missing RCU lock/unlock on ieee80211_rx_bss_info()

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On Tue, 2009-06-02 at 13:43 -0700, Luis R. Rodriguez wrote:

> Hm yeah, I failed to see it doesn't get the sta before allocating it
> and/or adding it. Is that right for this particular case? If the sta
> does exist we wouldn't have set up the things for it that
> ieee80211_ibss_add_sta() is setting.

Yeah, but that's a corner case we can only hit via this function racing
against itself, if that's even possible.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]