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

On 09/08/2026 12:21, Ruinskiy, Dima wrote:
> 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]>
> 
Tested-by: Moriya Kadosh <[email protected]>