* Error handling corner case found during audits
@ 2008-04-29 13:31 Alan Cox
2008-04-29 18:18 ` [PATCH] " Kok, Auke
0 siblings, 1 reply; 3+ messages in thread
From: Alan Cox @ 2008-04-29 13:31 UTC (permalink / raw)
To: auke-jan.h.kok, netdev
Not sure what should happen here.
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c
--- linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-28 11:36:49.000000000 +0100
+++ linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-18 16:42:41.000000000 +0100
@@ -494,6 +494,8 @@
for (i = 0; i < last_word - first_word + 1; i++) {
ret_val = e1000_read_nvm(hw, first_word + i, 1,
&eeprom_buff[i]);
+ /* ERROR: This path leaves eeprom_buf containing
+ old kernel bytes we then byteswap/return */
if (ret_val)
break;
}
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] Re: Error handling corner case found during audits 2008-04-29 13:31 Error handling corner case found during audits Alan Cox @ 2008-04-29 18:18 ` Kok, Auke 2008-05-06 16:40 ` Jeff Garzik 0 siblings, 1 reply; 3+ messages in thread From: Kok, Auke @ 2008-04-29 18:18 UTC (permalink / raw) To: Jeff Garzik; +Cc: NetDev, Alan Cox, Jesse Brandeburg Alan Cox wrote: > Not sure what should happen here. > > > diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c > --- linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-28 11:36:49.000000000 +0100 > +++ linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-18 16:42:41.000000000 +0100 > @@ -494,6 +494,8 @@ > for (i = 0; i < last_word - first_word + 1; i++) { > ret_val = e1000_read_nvm(hw, first_word + i, 1, > &eeprom_buff[i]); > + /* ERROR: This path leaves eeprom_buf containing > + old kernel bytes we then byteswap/return */ > if (ret_val) > break; > } either we fill the buffer with 0xff (the determined value for "empty eeprom"), or just kzalloc the buffer instead. This should be enough of a warning for the user that something is really wrong. Auke --- e1000e: don't return half-read eeprom on error On a read error, e1000e might have returned uninitialized block of eeprom data back to userspace. The convention is that 0xff is "empty", so mark the entire eeprom as empty in case of an error. Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com> --- diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c index b1b784a..8b04a42 100644 --- a/drivers/net/e1000e/ethtool.c +++ b/drivers/net/e1000e/ethtool.c @@ -510,8 +510,12 @@ static int e1000_get_eeprom(struct net_device *netdev, for (i = 0; i < last_word - first_word + 1; i++) { ret_val = e1000_read_nvm(hw, first_word + i, 1, &eeprom_buff[i]); - if (ret_val) + if (ret_val) { + /* a read error occurred, throw away the + * result */ + memset(eeprom_buff, 0xff, sizeof(eeprom_buff)); break; + } } } ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Re: Error handling corner case found during audits 2008-04-29 18:18 ` [PATCH] " Kok, Auke @ 2008-05-06 16:40 ` Jeff Garzik 0 siblings, 0 replies; 3+ messages in thread From: Jeff Garzik @ 2008-05-06 16:40 UTC (permalink / raw) To: Kok, Auke; +Cc: NetDev, Alan Cox, Jesse Brandeburg Kok, Auke wrote: > Alan Cox wrote: >> Not sure what should happen here. >> >> >> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c >> --- linux.vanilla-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-28 11:36:49.000000000 +0100 >> +++ linux-2.6.25-mm1/drivers/net/e1000e/ethtool.c 2008-04-18 16:42:41.000000000 +0100 >> @@ -494,6 +494,8 @@ >> for (i = 0; i < last_word - first_word + 1; i++) { >> ret_val = e1000_read_nvm(hw, first_word + i, 1, >> &eeprom_buff[i]); >> + /* ERROR: This path leaves eeprom_buf containing >> + old kernel bytes we then byteswap/return */ >> if (ret_val) >> break; >> } > > either we fill the buffer with 0xff (the determined value for "empty eeprom"), or > just kzalloc the buffer instead. This should be enough of a warning for the user > that something is really wrong. > > Auke > > --- > > e1000e: don't return half-read eeprom on error > > On a read error, e1000e might have returned uninitialized block of eeprom data > back to userspace. The convention is that 0xff is "empty", so mark the entire > eeprom as empty in case of an error. > > Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com> > > --- > diff --git a/drivers/net/e1000e/ethtool.c b/drivers/net/e1000e/ethtool.c > index b1b784a..8b04a42 100644 > --- a/drivers/net/e1000e/ethtool.c > +++ b/drivers/net/e1000e/ethtool.c > @@ -510,8 +510,12 @@ static int e1000_get_eeprom(struct net_device *netdev, > for (i = 0; i < last_word - first_word + 1; i++) { > ret_val = e1000_read_nvm(hw, first_word + i, 1, > &eeprom_buff[i]); > - if (ret_val) > + if (ret_val) { > + /* a read error occurred, throw away the > + * result */ > + memset(eeprom_buff, 0xff, sizeof(eeprom_buff)); > break; > + } > } > } applied ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-05-06 16:40 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-29 13:31 Error handling corner case found during audits Alan Cox 2008-04-29 18:18 ` [PATCH] " Kok, Auke 2008-05-06 16:40 ` Jeff Garzik
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).