Re: [ath9k-devel] smatch stuff: potential read past the end of the buffer

2010-12-20 Thread Senthil Balasubramanian 
On Mon, Dec 20, 2010 at 02:00:41PM +0530, Dan Carpenter wrote:
 Hello Vasanthakumar,
 
 Smatch complains that in linux-next 60e0c3a7 ath9k_hw: Eeeprom changes
 for AR9485 means there is a potential read past the end of the buffer
 int ar9300_eeprom_restore_internal().
 
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c +3381
   ar9300_eeprom_restore_internal(81)
   error: buffer overflow 'word' 2048 = 4099
 
 word is allocated with 2048 bytes:
 word = kzalloc(2048, GFP_KERNEL);
 
 length can be up to 4099:
 if ((!AR_SREV_9485(ah)  length = 1024) ||
 (AR_SREV_9485(ah)  length = (4 * 1024))) {
  ^
 
 so osize can be up to 4099 as well:
 osize = length;
 
 We're reading way past the end of the word array here:
 mchecksum = word[COMP_HDR_LEN + osize] |
 ^^
 (word[COMP_HDR_LEN + osize + 1]  8);
 
 I don't know how to fix this.  Can you take a look?
thanks for pointing this out. we shall look into this.
 
 regards,
 dan carpenter
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-wireless in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Re: [ath9k-devel] smatch stuff: potential read past the end of the buffer

2010-12-20 Thread Arend Van Spriel 
Hi Dan,

Why not use min() function?
index = min(COMP_HDR_LEN + osize, 2046);
mchecksum = word[index] |
(word[index + 1]  8);

Or would smatch miss this in its analysis?

Gr. AvS

From: linux-wireless-ow...@vger.kernel.org 
[linux-wireless-ow...@vger.kernel.org] On Behalf Of Dan Carpenter 
[erro...@gmail.com]
Sent: Monday, December 20, 2010 9:30 AM
To: Vasanthakumar Thiagarajan
Cc: linux-wirel...@vger.kernel.org; ath9k-devel@lists.ath9k.org
Subject: smatch stuff: potential read past the end of the buffer

Hello Vasanthakumar,

Smatch complains that in linux-next 60e0c3a7 ath9k_hw: Eeeprom changes
for AR9485 means there is a potential read past the end of the buffer
int ar9300_eeprom_restore_internal().

drivers/net/wireless/ath/ath9k/ar9003_eeprom.c +3381
ar9300_eeprom_restore_internal(81)
error: buffer overflow 'word' 2048 = 4099

word is allocated with 2048 bytes:
word = kzalloc(2048, GFP_KERNEL);

length can be up to 4099:
if ((!AR_SREV_9485(ah)  length = 1024) ||
(AR_SREV_9485(ah)  length = (4 * 1024))) {
 ^

so osize can be up to 4099 as well:
osize = length;

We're reading way past the end of the word array here:
mchecksum = word[COMP_HDR_LEN + osize] |
^^
(word[COMP_HDR_LEN + osize + 1]  8);

I don't know how to fix this.  Can you take a look?

regards,
dan carpenter

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Re: [ath9k-devel] smatch stuff: potential read past the end of the buffer

2010-12-20 Thread Arend Van Spriel 
Hi Dan,

Agreed. But maybe there is no usage scenario in which the boundary is actually 
crossed. Have to wait for ath9k developers to answer that.

Gr. AvS

From: Dan Carpenter [erro...@gmail.com]
Sent: Monday, December 20, 2010 1:42 PM
To: Arend Van Spriel
Cc: Vasanthakumar Thiagarajan; linux-wirel...@vger.kernel.org; 
ath9k-devel@lists.ath9k.org
Subject: Re: smatch stuff: potential read past the end of the buffer

On Mon, Dec 20, 2010 at 02:16:56AM -0800, Arend Van Spriel wrote:
 Hi Dan,

 Why not use min() function?
 index = min(COMP_HDR_LEN + osize, 2046);
 mchecksum = word[index] |
 (word[index + 1]  8);

 Or would smatch miss this in its analysis?

That would silence the warning, but is it the right fix?  I thought
maybe we should make word a larger buffer?

regards,
dan carpenter




___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

Re: [ath9k-devel] smatch stuff: potential read past the end of the buffer

2010-12-20 Thread Vasanthakumar Thiagarajan 
On Mon, Dec 20, 2010 at 02:00:41PM +0530, Dan Carpenter wrote:
 Hello Vasanthakumar,
 
 Smatch complains that in linux-next 60e0c3a7 ath9k_hw: Eeeprom changes
 for AR9485 means there is a potential read past the end of the buffer
 int ar9300_eeprom_restore_internal().
 
 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c +3381
   ar9300_eeprom_restore_internal(81)
   error: buffer overflow 'word' 2048 = 4099
 
 word is allocated with 2048 bytes:
 word = kzalloc(2048, GFP_KERNEL);
 
 length can be up to 4099:
 if ((!AR_SREV_9485(ah)  length = 1024) ||
 (AR_SREV_9485(ah)  length = (4 * 1024))) {

Yeah, this looks buggy, the eeprom data length for AR9485 is 1088
bytes only, I'll send out a patch, thanks for reporting this.

Vasanth
___
ath9k-devel mailing list
ath9k-devel@lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel