<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=koi8-r">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from text --><style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<meta content="text/html; charset=UTF-8">
<style type="text/css" style="">
<!--
p
{margin-top:0;
margin-bottom:0}
-->
</style>
<div dir="ltr">
<div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Helvetica,sans-serif">
<div>Hi Tony,</div>
<div><br>
</div>
<div>Thanks for the review!</div>
<div><br>
</div>
<div>Regarding e1000e: the return value of e1000_read_eeprom() is already properly checked.</div>
<div>Therefore, for e1000e, I am only submitting the endianness conversion cleanup as part</div>
<div>of a separate 'net-next' series. The functional fix for e1000 (missing return value check)</div>
<div>will be sent as a standalone patch for the 'net' tree.</div>
<div><br>
</div>
<div>I'm also removing the Fixes: tags from the endianness cleanup patches since, as you noted,</div>
<div>the uninitialized data is eventually overwritten by memcpy() and doesn't cause a functional</div>
<div>failure.</div>
<div><br>
</div>
<div>Best regards,</div>
<div>Daniil</div>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="x_divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Tony Nguyen <anthony.l.nguyen@intel.com><br>
<b>Sent:</b> Wednesday, March 25, 2026 2:27:12 AM<br>
<b>To:</b> Агалаков Даниил<br>
<b>Cc:</b> Przemek Kitszel; Andrew Lunn; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; lvc-project@linuxtesting.org; Исхаков Даниил; Разов Роман<br>
<b>Subject:</b> Re: [PATCH net 3/3] e1000e: fix endianness conversion of uninitialized words</font>
<div> </div>
</div>
</div>
<font size="2"><span style="font-size:10pt;">
<div class="PlainText"><br>
<br>
On 3/18/2026 5:05 AM, Agalakov Daniil wrote:<br>
> [Why]<br>
> In e1000_set_eeprom(), the eeprom_buff is allocated to hold a range of<br>
> words. However, only the boundary words (the first and the last) are<br>
> populated from the EEPROM if the write request is not word-aligned.<br>
> The words in the middle of the buffer remain uninitialized because they<br>
> are intended to be completely overwritten by the new data via memcpy().<br>
> <br>
> The previous implementation had a loop that performed le16_to_cpus()<br>
> on the entire buffer. This resulted in endianness conversion being<br>
> performed on uninitialized memory for all interior words.<br>
> <br>
> Fix this by converting the endianness only for the boundary words<br>
> immediately after they are successfully read from the EEPROM.<br>
> <br>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.<br>
> <br>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")<br>
<br>
AI Review reports:<br>
<br>
The commit message cites the initial git repository commit 1da177e4c3f4<br>
("Linux-2.6.12-rc2") from 2005 as the source of the bug. However, the<br>
e1000e driver wasn't introduced until 2007 in commit bc7f75fa9788<br>
("[E1000E]: New pci-express e1000 driver (currently for ICH9 devices<br>
only)"). While the e1000 driver did have this bug pattern in the initial<br>
commit, this patch fixes the e1000e driver, which is a separate driver.<br>
<br>
Should the Fixes: tag reference bc7f75fa9788 instead, since that's when<br>
the buggy pattern was first introduced in e1000e?<br>
<br>
Also, the same comment from the e1000 patch applies here. I think this <br>
patch should be split like the e1000 ones with the return value going to <br>
*-net and the endian to *-next.<br>
<br>
Thanks,<br>
Tony<br>
<br>
<br>
> Co-developed-by: Iskhakov Daniil <dish@amicon.ru><br>
> Signed-off-by: Iskhakov Daniil <dish@amicon.ru><br>
> Signed-off-by: Agalakov Daniil <ade@amicon.ru><br>
> ---<br>
> drivers/net/ethernet/intel/e1000e/ethtool.c | 19 ++++++++++++-------<br>
> 1 file changed, 12 insertions(+), 7 deletions(-)<br>
> <br>
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c b/drivers/net/ethernet/intel/e1000e/ethtool.c<br>
> index dbed30943ef4..a8b35ae41141 100644<br>
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c<br>
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c<br>
> @@ -583,20 +583,25 @@ static int e1000_set_eeprom(struct net_device *netdev,<br>
> /* need read/modify/write of first changed EEPROM word */<br>
> /* only the second byte of the word is being modified */<br>
> ret_val = e1000_read_nvm(hw, first_word, 1, &eeprom_buff[0]);<br>
> + if (ret_val)<br>
> + goto out;<br>
> +<br>
> + /* Device's eeprom is always little-endian, word addressable */<br>
> + le16_to_cpus(&eeprom_buff[0]);<br>
> +<br>
> ptr++;<br>
> }<br>
> - if (((eeprom->offset + eeprom->len) & 1) && (!ret_val))<br>
> + if ((eeprom->offset + eeprom->len) & 1) {<br>
> /* need read/modify/write of last changed EEPROM word */<br>
> /* only the first byte of the word is being modified */<br>
> ret_val = e1000_read_nvm(hw, last_word, 1,<br>
> &eeprom_buff[last_word - first_word]);<br>
> + if (ret_val)<br>
> + goto out;<br>
> <br>
> - if (ret_val)<br>
> - goto out;<br>
> -<br>
> - /* Device's eeprom is always little-endian, word addressable */<br>
> - for (i = 0; i < last_word - first_word + 1; i++)<br>
> - le16_to_cpus(&eeprom_buff[i]);<br>
> + /* Device's eeprom is always little-endian, word addressable */<br>
> + le16_to_cpus(&eeprom_buff[last_word - first_word]);<br>
> + }<br>
> <br>
> memcpy(ptr, bytes, eeprom->len);<br>
> <br>
<br>
</div>
</span></font>
</body>
</html>