* [PATCH] ath9k: Fix read buffer overflow
@ 2009-08-07 21:50 Roel Kluin
2009-08-10 20:26 ` John W. Linville
0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-08-07 21:50 UTC (permalink / raw)
To: Jouni Malinen, linux-wireless, ath9k-devel, Andrew Morton
Prevent a read of powInfo[-1] in the first iteration.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
The last hunk I already sent in a previous patch.
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index a2fda70..ef4bf89 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -150,10 +150,10 @@ static void ath9k_hw_get_legacy_target_powers(struct ath_hw *ah,
IS_CHAN_2GHZ(chan))) {
matchIndex = i;
break;
- } else if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
- IS_CHAN_2GHZ(chan))) &&
- (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
- IS_CHAN_2GHZ(chan)))) {
+ } else if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
+ IS_CHAN_2GHZ(chan)) && i > 0 &&
+ freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
+ IS_CHAN_2GHZ(chan))) {
lowIndex = i - 1;
break;
}
@@ -268,10 +268,10 @@ static void ath9k_hw_get_target_powers(struct ath_hw *ah,
matchIndex = i;
break;
} else
- if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
- IS_CHAN_2GHZ(chan))) &&
- (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
- IS_CHAN_2GHZ(chan)))) {
+ if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
+ IS_CHAN_2GHZ(chan)) && i > 0 &&
+ freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
+ IS_CHAN_2GHZ(chan))) {
lowIndex = i - 1;
break;
}
@@ -460,7 +460,7 @@ static int ath9k_hw_4k_check_eeprom(struct ath_hw *ah)
integer = swab32(eep->modalHeader.antCtrlCommon);
eep->modalHeader.antCtrlCommon = integer;
- for (i = 0; i < AR5416_MAX_CHAINS; i++) {
+ for (i = 0; i < AR5416_EEP4K_MAX_CHAINS; i++) {
integer = swab32(eep->modalHeader.antCtrlChain[i]);
eep->modalHeader.antCtrlChain[i] = integer;
}
@@ -914,7 +914,7 @@ static void ath9k_hw_set_4k_power_per_rate_table(struct ath_hw *ah,
ctlMode, numCtlModes, isHt40CtlMode,
(pCtlMode[ctlMode] & EXT_ADDITIVE));
- for (i = 0; (i < AR5416_NUM_CTLS) &&
+ for (i = 0; (i < AR5416_EEP4K_NUM_CTLS) &&
pEepData->ctlIndex[i]; i++) {
DPRINTF(ah->ah_sc, ATH_DBG_EEPROM,
" LOOP-Ctlidx %d: cfgCtl 0x%2.2x "
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k: Fix read buffer overflow
2009-08-07 21:50 [PATCH] ath9k: Fix read buffer overflow Roel Kluin
@ 2009-08-10 20:26 ` John W. Linville
2009-08-11 6:49 ` Roel Kluin
0 siblings, 1 reply; 6+ messages in thread
From: John W. Linville @ 2009-08-10 20:26 UTC (permalink / raw)
To: Roel Kluin; +Cc: Jouni Malinen, linux-wireless, ath9k-devel, Andrew Morton
On Fri, Aug 07, 2009 at 11:50:00PM +0200, Roel Kluin wrote:
> Prevent a read of powInfo[-1] in the first iteration.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> The last hunk I already sent in a previous patch.
The last two hunks, apparently -- neither of them apply.
Why on earth would you randomly include those hunks here? Especially
if you already sent them?
Listen, I'm glad you are there to do these various clean-ups.
But honestly your patches seem to always have some problem that
requires manual intervention to apply them. You make my life hard,
and you slow me down. That makes John a grumpy boy.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ath9k: Fix read buffer overflow
2009-08-10 20:26 ` John W. Linville
@ 2009-08-11 6:49 ` Roel Kluin
2009-08-11 18:25 ` John W. Linville
0 siblings, 1 reply; 6+ messages in thread
From: Roel Kluin @ 2009-08-11 6:49 UTC (permalink / raw)
To: John W. Linville
Cc: Jouni Malinen, linux-wireless, ath9k-devel, Andrew Morton
Prevent a read of powInfo[-1] in the first iteration.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index ce0e86c..e67db2c 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -150,10 +150,10 @@ static void ath9k_hw_get_legacy_target_powers(struct ath_hw *ah,
IS_CHAN_2GHZ(chan))) {
matchIndex = i;
break;
- } else if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
- IS_CHAN_2GHZ(chan))) &&
- (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
- IS_CHAN_2GHZ(chan)))) {
+ } else if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
+ IS_CHAN_2GHZ(chan)) && i > 0 &&
+ freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
+ IS_CHAN_2GHZ(chan))) {
lowIndex = i - 1;
break;
}
@@ -268,10 +268,10 @@ static void ath9k_hw_get_target_powers(struct ath_hw *ah,
matchIndex = i;
break;
} else
- if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
- IS_CHAN_2GHZ(chan))) &&
- (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
- IS_CHAN_2GHZ(chan)))) {
+ if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
+ IS_CHAN_2GHZ(chan)) && i > 0 &&
+ freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
+ IS_CHAN_2GHZ(chan))) {
lowIndex = i - 1;
break;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k: Fix read buffer overflow
2009-08-11 6:49 ` Roel Kluin
@ 2009-08-11 18:25 ` John W. Linville
2009-08-20 14:52 ` John W. Linville
0 siblings, 1 reply; 6+ messages in thread
From: John W. Linville @ 2009-08-11 18:25 UTC (permalink / raw)
To: Roel Kluin
Cc: Jouni Malinen, linux-wireless, ath9k-devel, Andrew Morton,
m.sujith, mcgrof
Comments from the ath9k crowd?
On Tue, Aug 11, 2009 at 08:49:08AM +0200, Roel Kluin wrote:
> Prevent a read of powInfo[-1] in the first iteration.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
> index ce0e86c..e67db2c 100644
> --- a/drivers/net/wireless/ath/ath9k/eeprom.c
> +++ b/drivers/net/wireless/ath/ath9k/eeprom.c
> @@ -150,10 +150,10 @@ static void ath9k_hw_get_legacy_target_powers(struct ath_hw *ah,
> IS_CHAN_2GHZ(chan))) {
> matchIndex = i;
> break;
> - } else if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> - IS_CHAN_2GHZ(chan))) &&
> - (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> - IS_CHAN_2GHZ(chan)))) {
> + } else if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> + IS_CHAN_2GHZ(chan)) && i > 0 &&
> + freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> + IS_CHAN_2GHZ(chan))) {
> lowIndex = i - 1;
> break;
> }
> @@ -268,10 +268,10 @@ static void ath9k_hw_get_target_powers(struct ath_hw *ah,
> matchIndex = i;
> break;
> } else
> - if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> - IS_CHAN_2GHZ(chan))) &&
> - (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> - IS_CHAN_2GHZ(chan)))) {
> + if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> + IS_CHAN_2GHZ(chan)) && i > 0 &&
> + freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> + IS_CHAN_2GHZ(chan))) {
> lowIndex = i - 1;
> break;
> }
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k: Fix read buffer overflow
2009-08-11 18:25 ` John W. Linville
@ 2009-08-20 14:52 ` John W. Linville
2009-08-24 23:34 ` Luis R. Rodriguez
0 siblings, 1 reply; 6+ messages in thread
From: John W. Linville @ 2009-08-20 14:52 UTC (permalink / raw)
To: Roel Kluin
Cc: Jouni Malinen, linux-wireless, ath9k-devel, Andrew Morton,
m.sujith, mcgrof
Anybody?
On Tue, Aug 11, 2009 at 02:25:14PM -0400, John W. Linville wrote:
> Comments from the ath9k crowd?
>
> On Tue, Aug 11, 2009 at 08:49:08AM +0200, Roel Kluin wrote:
> > Prevent a read of powInfo[-1] in the first iteration.
> >
> > Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> > ---
> > diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
> > index ce0e86c..e67db2c 100644
> > --- a/drivers/net/wireless/ath/ath9k/eeprom.c
> > +++ b/drivers/net/wireless/ath/ath9k/eeprom.c
> > @@ -150,10 +150,10 @@ static void ath9k_hw_get_legacy_target_powers(struct ath_hw *ah,
> > IS_CHAN_2GHZ(chan))) {
> > matchIndex = i;
> > break;
> > - } else if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> > - IS_CHAN_2GHZ(chan))) &&
> > - (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> > - IS_CHAN_2GHZ(chan)))) {
> > + } else if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> > + IS_CHAN_2GHZ(chan)) && i > 0 &&
> > + freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> > + IS_CHAN_2GHZ(chan))) {
> > lowIndex = i - 1;
> > break;
> > }
> > @@ -268,10 +268,10 @@ static void ath9k_hw_get_target_powers(struct ath_hw *ah,
> > matchIndex = i;
> > break;
> > } else
> > - if ((freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> > - IS_CHAN_2GHZ(chan))) &&
> > - (freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> > - IS_CHAN_2GHZ(chan)))) {
> > + if (freq < ath9k_hw_fbin2freq(powInfo[i].bChannel,
> > + IS_CHAN_2GHZ(chan)) && i > 0 &&
> > + freq > ath9k_hw_fbin2freq(powInfo[i - 1].bChannel,
> > + IS_CHAN_2GHZ(chan))) {
> > lowIndex = i - 1;
> > break;
> > }
> >
>
> --
> John W. Linville Someday the world will need a hero, and you
> linville@tuxdriver.com might be all we have. Be ready.
> --
> 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
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ath9k: Fix read buffer overflow
2009-08-20 14:52 ` John W. Linville
@ 2009-08-24 23:34 ` Luis R. Rodriguez
0 siblings, 0 replies; 6+ messages in thread
From: Luis R. Rodriguez @ 2009-08-24 23:34 UTC (permalink / raw)
To: John W. Linville
Cc: Roel Kluin, Jouni Malinen, linux-wireless, ath9k-devel,
Andrew Morton, m.sujith
On Thu, Aug 20, 2009 at 7:52 AM, John W. Linville<linville@tuxdriver.com> wrote:
> Anybody?
Sorry for the delay,
Acked-by: Luis R. Rodriguez <lrodriguez@atheros.com>
This is actually pretty sloppy existing code and I'd prefer to see
this nasty POS code rewritten to avoid such nasty checks from the
start. Also notice how both ath9k_hw_get_legacy_target_powers() and
ath9k_hw_get_target_powers() do exactly the same, except they use a
different name for the bool, a different structure for the calibrated
power targets (array size changes on one element of the struct). But
this patch also fixes another not-noted potential negative rade index
access: lowIndex could be -1 under a special circumstance and this
would prevent that negative index access as well on powInfo[lowIndex].
So although this probably just does not happen right now its safer to
have a fix for two of these theoretical negative array index access
than nothing at hand; a proper rewrite of these two routines as I want
it would require quite a few changes here and more testing. Mentally
lets add that to the TODO list..
Luis
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-08-24 23:34 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-07 21:50 [PATCH] ath9k: Fix read buffer overflow Roel Kluin
2009-08-10 20:26 ` John W. Linville
2009-08-11 6:49 ` Roel Kluin
2009-08-11 18:25 ` John W. Linville
2009-08-20 14:52 ` John W. Linville
2009-08-24 23:34 ` Luis R. Rodriguez
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).