Re: [Intel-wired-lan] [PATCH] e1000e: fix incorrect modified flag check in e1000_read_nvm_spt()

["Ruinskiy, Dima" <dima.ruinskiy@intel.com>]

On 07/08/2026 13:56, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
>> Of Ivy Lopez
>> Sent: Thursday, August 6, 2026 7:25 PM
>> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>
>> Cc: andrew+netdev@lunn.ch; davem@davemloft.net; edumazet@google.com;
>> kuba@kernel.org; pabeni@redhat.com; intel-wired-lan@lists.osuosl.org;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Ivy Lopez
>> <skunkolee@gmail.com>
>> 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 <skunkolee@gmail.com>
>> ---
>>   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 <aleksandr.loktionov@intel.com>
> 
Reviewed-by: Dima Ruinskiy <dima.ruinskiy@intel.com>