linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).