* [PATCH] ath9k: Prevent read buffer overflow
@ 2009-08-11 18:40 Roel Kluin
2009-08-12 11:58 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 4+ messages in thread
From: Roel Kluin @ 2009-08-11 18:40 UTC (permalink / raw)
To: Luis R. Rodriguez, linux-wireless, ath9k-devel, Andrew Morton
Prevent a read from valid_rate_index[] with a negative index
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Maybe we should add this?
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index ba06e78..a67b7f6 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1458,7 +1458,7 @@ static void ath_rc_init(struct ath_softc *sc,
ath_rc_priv->rate_max_phy = ath_rc_priv->valid_phy_rateidx[i][j-1];
}
ASSERT(ath_rc_priv->rate_table_size <= RATE_TABLE_SIZE);
- ASSERT(k <= RATE_TABLE_SIZE);
+ ASSERT(k <= RATE_TABLE_SIZE && k >= 4);
ath_rc_priv->max_valid_rate = k;
ath_rc_sort_validrates(rate_table, ath_rc_priv);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ath9k: Prevent read buffer overflow
2009-08-11 18:40 [PATCH] ath9k: Prevent read buffer overflow Roel Kluin
@ 2009-08-12 11:58 ` Vasanthakumar Thiagarajan
2009-08-12 12:35 ` roel kluin
0 siblings, 1 reply; 4+ messages in thread
From: Vasanthakumar Thiagarajan @ 2009-08-12 11:58 UTC (permalink / raw)
To: Roel Kluin; +Cc: Luis Rodriguez, linux-wireless, ath9k-devel, Andrew Morton
On Wed, Aug 12, 2009 at 12:10:30AM +0530, Roel Kluin wrote:
> Prevent a read from valid_rate_index[] with a negative index
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Maybe we should add this?
>
> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
> index ba06e78..a67b7f6 100644
> --- a/drivers/net/wireless/ath/ath9k/rc.c
> +++ b/drivers/net/wireless/ath/ath9k/rc.c
> @@ -1458,7 +1458,7 @@ static void ath_rc_init(struct ath_softc *sc,
> ath_rc_priv->rate_max_phy = ath_rc_priv->valid_phy_rateidx[i][j-1];
> }
> ASSERT(ath_rc_priv->rate_table_size <= RATE_TABLE_SIZE);
> - ASSERT(k <= RATE_TABLE_SIZE);
> + ASSERT(k <= RATE_TABLE_SIZE && k >= 4);
NACK, k is initialized to 0 in the for loop few lines above this
ASSERT.
Vasanth
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ath9k: Prevent read buffer overflow
2009-08-12 11:58 ` Vasanthakumar Thiagarajan
@ 2009-08-12 12:35 ` roel kluin
2009-08-12 12:42 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 4+ messages in thread
From: roel kluin @ 2009-08-12 12:35 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan
Cc: Luis Rodriguez, linux-wireless, ath9k-devel, Andrew Morton
On Wed, Aug 12, 2009 at 1:58 PM, Vasanthakumar
Thiagarajan<vasanth@atheros.com> wrote:
> On Wed, Aug 12, 2009 at 12:10:30AM +0530, Roel Kluin wrote:
>> Prevent a read from valid_rate_index[] with a negative index
>>
>> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
>> ---
>> Maybe we should add this?
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
>> index ba06e78..a67b7f6 100644
>> --- a/drivers/net/wireless/ath/ath9k/rc.c
>> +++ b/drivers/net/wireless/ath/ath9k/rc.c
>> @@ -1458,7 +1458,7 @@ static void ath_rc_init(struct ath_softc *sc,
>> ath_rc_priv->rate_max_phy = ath_rc_priv->valid_phy_rateidx[i][j-1];
>> }
>> ASSERT(ath_rc_priv->rate_table_size <= RATE_TABLE_SIZE);
>> - ASSERT(k <= RATE_TABLE_SIZE);
>> + ASSERT(k <= RATE_TABLE_SIZE && k >= 4);
>
>
> NACK, k is initialized to 0 in the for loop few lines above this
> ASSERT.
>
> Vasanth
You could be right, but please explain, I don't understand:
k can only increment if ath_rc_priv->valid_phy_ratecnt[i] != 0
for i = 0 to WLAN_RC_PHY_MAX,
A few lines above these `ath_rc_priv->valid_phy_ratecnt[]'
are initialized to 0.
Say there was no working rate, and we call ath_rc_init_validrates(),
then in ath_rc_init_validrates()
ath_rc_priv->valid_phy_ratecnt[] can be initialized in this loop:
for (i = 0; i < rate_table->rate_cnt; i++) {
...
}
but where is this rate_cnt initialized?
[roel@zoinx linux-git]$ git grep rate_cnt
drivers/net/wireless/ath/ath9k/debug.c: max = 80 +
sc->cur_rate_table->rate_cnt * 64;
drivers/net/wireless/ath/ath9k/debug.c: for (i = 0; i <
sc->cur_rate_table->rate_cnt; i++) {
drivers/net/wireless/ath/ath9k/main.c: if (rate_table->rate_cnt > ATH_RATE_MAX)
drivers/net/wireless/ath/ath9k/main.c: maxrates = rate_table->rate_cnt;
drivers/net/wireless/ath/ath9k/rc.c: for (i = 0; i <
rate_table->rate_cnt; i++) {
drivers/net/wireless/ath/ath9k/rc.c: for (j = 0; j <
rate_table->rate_cnt; j++) {
drivers/net/wireless/ath/ath9k/rc.c: for (j = 0; j <
rate_table->rate_cnt; j++) {
drivers/net/wireless/ath/ath9k/rc.c: if ((tx_rate < 0) || (tx_rate
> rate_table->rate_cnt))
drivers/net/wireless/ath/ath9k/rc.h: int rate_cnt;
Roel
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ath9k: Prevent read buffer overflow
2009-08-12 12:35 ` roel kluin
@ 2009-08-12 12:42 ` Vasanthakumar Thiagarajan
0 siblings, 0 replies; 4+ messages in thread
From: Vasanthakumar Thiagarajan @ 2009-08-12 12:42 UTC (permalink / raw)
To: roel kluin
Cc: Vasanth Thiagarajan, Luis Rodriguez, linux-wireless, ath9k-devel,
Andrew Morton
On Wed, Aug 12, 2009 at 06:05:55PM +0530, roel kluin wrote:
> On Wed, Aug 12, 2009 at 1:58 PM, Vasanthakumar
> Thiagarajan<vasanth@atheros.com> wrote:
> > On Wed, Aug 12, 2009 at 12:10:30AM +0530, Roel Kluin wrote:
> >> Prevent a read from valid_rate_index[] with a negative index
> >>
> >> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> >> ---
> >> Maybe we should add this?
> >>
> >> diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
> >> index ba06e78..a67b7f6 100644
> >> --- a/drivers/net/wireless/ath/ath9k/rc.c
> >> +++ b/drivers/net/wireless/ath/ath9k/rc.c
> >> @@ -1458,7 +1458,7 @@ static void ath_rc_init(struct ath_softc *sc,
> >> ath_rc_priv->rate_max_phy = ath_rc_priv->valid_phy_rateidx[i][j-1];
> >> }
> >> ASSERT(ath_rc_priv->rate_table_size <= RATE_TABLE_SIZE);
> >> - ASSERT(k <= RATE_TABLE_SIZE);
> >> + ASSERT(k <= RATE_TABLE_SIZE && k >= 4);
> >
> >
> > NACK, k is initialized to 0 in the for loop few lines above this
> > ASSERT.
> >
> > Vasanth
>
> but where is this rate_cnt initialized?
from the static rate table for the respective mode. You can find
these tables in the begining of rc.c.
Vasanth
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-08-12 12:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-11 18:40 [PATCH] ath9k: Prevent read buffer overflow Roel Kluin
2009-08-12 11:58 ` Vasanthakumar Thiagarajan
2009-08-12 12:35 ` roel kluin
2009-08-12 12:42 ` Vasanthakumar Thiagarajan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).