Re: Suspicious RCU usage in mac80211

[Mohammed Shafi <shafi.wireless@gmail.com>]

Hi Larry,

On Fri, May 4, 2012 at 11:47 AM, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> On 05/03/2012 01:38 PM, Johannes Berg wrote:
>>
>> diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
>> index 5b7053c..40d3ff4 100644
>> --- a/net/mac80211/agg-tx.c
>> +++ b/net/mac80211/agg-tx.c
>> @@ -421,16 +421,22 @@ static void
>> sta_tx_agg_session_timer_expired(unsigned long data)
>>        struct tid_ampdu_tx *tid_tx;
>>        unsigned long timeout;
>>
>> -       tid_tx = rcu_dereference_protected_tid_tx(sta, *ptid);
>> -       if (!tid_tx)
>> +       rcu_read_lock();
>> +       tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[*ptid]);
>> +       if (!tid_tx) {
>> +               rcu_read_unlock();
>>                return;
>> +       }
>>
>>        timeout = tid_tx->last_tx + TU_TO_JIFFIES(tid_tx->timeout);
>>        if (time_is_after_jiffies(timeout)) {
>>                mod_timer(&tid_tx->session_timer, timeout);
>> +               rcu_read_unlock();
>>                return;
>>        }
>>
>> +       rcu_read_unlock();
>> +
>>  #ifdef CONFIG_MAC80211_HT_DEBUG
>>        printk(KERN_DEBUG "tx session timer expired on tid %d\n",
>> (u16)*ptid);
>>  #endif
>
>
> This patch is the same as was proposed by Mohammed on May 1. I must have
> messed up the testing as I thought it failed; however, it is now working.

honestly i am not sure about the technical details, but Johannes patch
seems to follow almost similar
what's is done in 'sta_addba_resp_timer_expired'. while thought that
is being also with mutex under hold
but the difference is  the lock is unhold before
ieee80211_stop_tx_ba_session is called.

>
> Mohammed - I plan on submitting this to John with a "From:" from you along
> with your s-o-b. Is that OK. It will be submitted for inclusion in 3.5.
>
> Larry
>


-- 
thanks,
shafi