Re: [Intel-wired-lan] [PATCH] e1000e: fix incorrect modified flag check in e1000_read_nvm_spt()
"Ruinskiy, Dima" <[email protected]>
| Newsgroups | org.osuosl.intel-wired-lan,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
On 07/08/2026 13:56, Loktionov, Aleksandr wrote: > > >> -----Original Message----- >> From: Intel-wired-lan <[email protected]> On Behalf >> Of Ivy Lopez >> Sent: Thursday, August 6, 2026 7:25 PM >> To: Nguyen, Anthony L <[email protected]>; Kitszel, >> Przemyslaw <[email protected]> >> Cc: [email protected]; [email protected]; [email protected]; >> [email protected]; [email protected]; [email protected]; >> [email protected]; [email protected]; Ivy Lopez >> <[email protected]> >> Subject: [Intel-wired-lan] [PATCH] e1000e: fix incorrect modified flag >> check in e1000_read_nvm_spt() >> >> e1000_read_nvm_spt() reads two adjacent 16-bit NVM words as one 32-bit >> dword for efficiency. When deciding whether to use the shadow RAM >> value for the high word (data[i + 1]), it incorrectly checks the >> "modified" flag of the low word (shadow_ram[offset + i]) instead of >> the high word's own flag (shadow_ram[offset + i + 1]). >> >> This can cause silent NVM shadow RAM corruption: >> - if only the high word is marked modified, its pending write is >> lost and the stale flash value is returned instead >> - if only the low word is marked modified, the high word is >> incorrectly overwritten with the low word's shadow value >> >> This function is used on all PCH chips from SPT onward (SPT, CNP, TGP, >> ADP, MTP, etc). The bug has been present since the dword-read >> optimization was introduced for these chips. >> >> Note: this was found via code review while investigating a separate >> reproducible packet loss issue on I219-LM (SPT+ PCH). Testing showed >> this fix does not resolve that particular symptom, but the flag check >> is still a genuine logic bug that can corrupt shadow RAM contents and >> should be fixed regardless. >> >> Fixes: 79849ebc0e06 ("e1000e: initial support for i219") >> Signed-off-by: Ivy Lopez <[email protected]> >> --- >> drivers/net/ethernet/intel/e1000e/ich8lan.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c >> b/drivers/net/ethernet/intel/e1000e/ich8lan.c >> index aa90e0ce8aca..87cd1cdc6f10 100644 >> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c >> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c >> @@ -3473,7 +3473,7 @@ static s32 e1000_read_nvm_spt(struct e1000_hw >> *hw, u16 offset, u16 words, >> dev_spec->shadow_ram[offset + i].value; >> else >> data[i] = (u16)(dword & 0xFFFF); >> - if (dev_spec->shadow_ram[offset + i].modified) >> + if (dev_spec->shadow_ram[offset + i + >> 1].modified) >> data[i + 1] = >> dev_spec->shadow_ram[offset + i + >> 1].value; >> else >> -- >> 2.55.0 > > Reviewed-by: Aleksandr Loktionov <[email protected]> > Reviewed-by: Dima Ruinskiy <[email protected]>