2009-07-02 21:53:20

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH] mac80211: minstrel: avoid accessing negative indices in rix_to_ndx()

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.

Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/rc80211_minstrel.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index b218b98..e2dd248 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -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 || mi->r[i].rix != rix);
return i;
}

@@ -181,6 +181,9 @@ minstrel_tx_status(void *priv, struct ieee80211_supported_band *sband,
break;

ndx = rix_to_ndx(mi, ar[i].idx);
+ if (ndx < 0)
+ continue;
+
mi->r[ndx].attempts += ar[i].count;

if ((i != IEEE80211_TX_MAX_RATES - 1) && (ar[i + 1].idx < 0))
--
1.6.0.4



2009-07-03 00:16:11

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: avoid accessing negative indices in rix_to_ndx()

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.
>
> Signed-off-by: Luciano Coelho <[email protected]>
> ---
> net/mac80211/rc80211_minstrel.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
> index b218b98..e2dd248 100644
> --- a/net/mac80211/rc80211_minstrel.c
> +++ b/net/mac80211/rc80211_minstrel.c
> @@ -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 || mi->r[i].rix != rix);
I believe this line could be changed to WARN_ON(i < 0), because
(mi->r[i].rix != rix) will never be true unless i < 0.
Probably not that important though.

Acked-by: Felix Fietkau <[email protected]>

- Felix

2009-07-03 05:17:11

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] mac80211: minstrel: avoid accessing negative indices in rix_to_ndx()

ext Felix Fietkau wrote:
> 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.
>>
>> Signed-off-by: Luciano Coelho <[email protected]>
>> ---
>> net/mac80211/rc80211_minstrel.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
>> index b218b98..e2dd248 100644
>> --- a/net/mac80211/rc80211_minstrel.c
>> +++ b/net/mac80211/rc80211_minstrel.c
>> @@ -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 || mi->r[i].rix != rix);
>>
> I believe this line could be changed to WARN_ON(i < 0), because
> (mi->r[i].rix != rix) will never be true unless i < 0.
> Probably not that important though.
>

That's true. I'll change the code and send v2. It's not that
important, but it is code that will never be reached, so it should be
removed.

> Acked-by: Felix Fietkau <[email protected]>
>

Thanks!

--
Cheers,
Luca.