Re: [PATCH v2] mac80211: minstrel: avoid accessing negative indices in rix_to_ndx()

[Luciano Coelho <luciano.coelho@nokia.com>]

ext Johannes Berg wrote:
> On Fri, 2009-07-03 at 08:25 +0300, Luciano Coelho wrote:
>   
>> If rix is not found in mi->r[], i will become -1 after the loop.  This value
>> is eventually used to access arrays, so we were accessing arrays with a
>> negative index, which is obviously not what we want to do.  This patch fixes
>> this potential problem.
>>     
>
> This seems odd -- are you or are you not saying that this can happen in
> normal operation?
>   

This should *not* happen in normal operation, but it was happening due 
to a bug elsewhere (which has already been fixed).

>> @@ -66,7 +66,7 @@ rix_to_ndx(struct minstrel_sta_info *mi, int rix)
>>  	for (i = rix; i >= 0; i--)
>>  		if (mi->r[i].rix == rix)
>>  			break;
>> -	WARN_ON(mi->r[i].rix != rix);
>> +	WARN_ON(i < 0);
>>  	return i;
>>     
>
> If it can, this warning seems wrong.
>   

We were reaching this WARN_ON on our device some time ago, before we 
backported some other minstrel fixes from upstream.  So this patch 
doesn't fix an *active* bug, but it makes things more robust in case 
there is a bug elsewhere.  It is not good at all to access mi->r[-1], 
especially the code later which writes to this array and could 
potentially overwrite somebody else's memory.

-- 
Cheers,
Luca.