* [ath9k-devel] [PATCH] ath9k: Prevent read buffer overflow @ 2009-08-11 18:40 ` Roel Kluin 0 siblings, 0 replies; 8+ messages in thread From: Roel Kluin @ 2009-08-11 18:40 UTC (permalink / raw) To: ath9k-devel 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] 8+ messages in thread
* [PATCH] ath9k: Prevent read buffer overflow @ 2009-08-11 18:40 ` Roel Kluin 0 siblings, 0 replies; 8+ 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] 8+ messages in thread
* [ath9k-devel] [PATCH] ath9k: Prevent read buffer overflow 2009-08-11 18:40 ` Roel Kluin @ 2009-08-12 11:58 ` Vasanthakumar Thiagarajan -1 siblings, 0 replies; 8+ messages in thread From: Vasanthakumar Thiagarajan @ 2009-08-12 11:58 UTC (permalink / raw) To: ath9k-devel 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] 8+ messages in thread
* Re: [PATCH] ath9k: Prevent read buffer overflow @ 2009-08-12 11:58 ` Vasanthakumar Thiagarajan 0 siblings, 0 replies; 8+ messages in thread From: Vasanthakumar Thiagarajan @ 2009-08-12 11:58 UTC (permalink / raw) To: Roel Kluin Cc: Luis Rodriguez, linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org, 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] 8+ messages in thread
* [ath9k-devel] [PATCH] ath9k: Prevent read buffer overflow 2009-08-12 11:58 ` Vasanthakumar Thiagarajan @ 2009-08-12 12:35 ` roel kluin -1 siblings, 0 replies; 8+ messages in thread From: roel kluin @ 2009-08-12 12:35 UTC (permalink / raw) To: ath9k-devel 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 at 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] 8+ messages in thread
* Re: [PATCH] ath9k: Prevent read buffer overflow @ 2009-08-12 12:35 ` roel kluin 0 siblings, 0 replies; 8+ messages in thread From: roel kluin @ 2009-08-12 12:35 UTC (permalink / raw) To: Vasanthakumar Thiagarajan Cc: Luis Rodriguez, linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org, 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] 8+ messages in thread
* [ath9k-devel] [PATCH] ath9k: Prevent read buffer overflow 2009-08-12 12:35 ` roel kluin @ 2009-08-12 12:42 ` Vasanthakumar Thiagarajan -1 siblings, 0 replies; 8+ messages in thread From: Vasanthakumar Thiagarajan @ 2009-08-12 12:42 UTC (permalink / raw) To: ath9k-devel 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] 8+ messages in thread
* Re: [PATCH] ath9k: Prevent read buffer overflow @ 2009-08-12 12:42 ` Vasanthakumar Thiagarajan 0 siblings, 0 replies; 8+ messages in thread From: Vasanthakumar Thiagarajan @ 2009-08-12 12:42 UTC (permalink / raw) To: roel kluin Cc: Vasanth Thiagarajan, Luis Rodriguez, linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org, 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] 8+ messages in thread
end of thread, other threads:[~2009-08-12 12:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-08-11 18:40 [ath9k-devel] [PATCH] ath9k: Prevent read buffer overflow Roel Kluin 2009-08-11 18:40 ` Roel Kluin 2009-08-12 11:58 ` [ath9k-devel] " Vasanthakumar Thiagarajan 2009-08-12 11:58 ` Vasanthakumar Thiagarajan 2009-08-12 12:35 ` [ath9k-devel] " roel kluin 2009-08-12 12:35 ` roel kluin 2009-08-12 12:42 ` [ath9k-devel] " Vasanthakumar Thiagarajan 2009-08-12 12:42 ` Vasanthakumar Thiagarajan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.