| Message ID | 4A81BB1E.6040904@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html