Skip to contentSkip to conversation

ath9k: Fix read buffer overflow

PATCH New Roel Kluin 3 participants 6 messages Aug 7, 2009 – Aug 24, 2009 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Prevent a read of powInfo[-1] in the first iteration.

---
The last hunk I already sent in a previous patch.

drivers/net/wireless/ath/ath9k/eeprom.c +10 -10
index a2fda70..ef4bf89 100644
@@ -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 "
#1 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Quoted 5 lines · Roel Kluin

Prevent a read of powInfo[-1] in the first iteration.

---
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

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.
author
#2 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Prevent a read of powInfo[-1] in the first iteration.

---

drivers/net/wireless/ath/ath9k/eeprom.c +8 -8
index ce0e86c..e67db2c 100644
@@ -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;
 				}
#3 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Comments from the ath9k crowd?

Quoted 39 lines · Roel Kluin

Prevent a read of powInfo[-1] in the first iteration.

---

drivers/net/wireless/ath/ath9k/eeprom.c +8 -8
index ce0e86c..e67db2c 100644
@@ -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;
 				}
Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.
#4 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Anybody?

Quoted 51 lines · John W. Linville

Comments from the ath9k crowd?

Quoted 39 lines · Roel Kluin

Prevent a read of powInfo[-1] in the first iteration.

---

drivers/net/wireless/ath/ath9k/eeprom.c +8 -8
index ce0e86c..e67db2c 100644
@@ -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;
 				}
Signature · 6 lines
John W. Linville		Someday the world will need a hero, and you
linville at 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 at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.
#5 lore raw

PATCH 1/1 ath9k: Fix read buffer overflow

Quoted 1 line · John W. Linville

Anybody?

Sorry for the delay,

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