netdev - Re: [PATCH v2 2/2] e1000e: ignore factory-default checksum value on TGP platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <431c1aaa-304d-4291-97f8-c092a6bee884@ursu.me>
Date: Tue, 24 Jun 2025 21:17:44 +0300
From: Vlad URSU <vlad@...u.me>
To: Jacek Kowalski <jacek@...ekk.info>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] e1000e: ignore factory-default checksum value on
 TGP platform

On 23.06.2025 19:18, Jacek Kowalski wrote:
> Vlad,
> 
> could you verify that the following patch works for you?
> 
>> diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
>> index 8294a7c4f122..01696eb8dace 100644
>> --- a/drivers/net/ethernet/intel/e1000e/defines.h
>> +++ b/drivers/net/ethernet/intel/e1000e/defines.h
>> @@ -637,6 +637,7 @@
>>   
>>   /* For checksumming, the sum of all words in the NVM should equal 0xBABA. */
>>   #define NVM_SUM                    0xBABA
>> +#define NVM_SUM_FACTORY_DEFAULT    0xFFFF
>>   
>>   /* PBA (printed board assembly) number words */
>>   #define NVM_PBA_OFFSET_0           8
>> diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c b/drivers/net/ethernet/intel/e1000e/nvm.c
>> index e609f4df86f4..37cbf9236d84 100644
>> --- a/drivers/net/ethernet/intel/e1000e/nvm.c
>> +++ b/drivers/net/ethernet/intel/e1000e/nvm.c
>> @@ -558,6 +558,11 @@ s32 e1000e_validate_nvm_checksum_generic(struct e1000_hw *hw)
>>   		checksum += nvm_data;
>>   	}
>>   
>> +	if (hw->mac.type == e1000_pch_tgp && checksum == (u16)NVM_SUM_FACTORY_DEFAULT) {
>> +		e_dbg("Factory-default NVM Checksum on TGP platform - ignoring\n");
>> +		return 0;
>> +	}
>> +
>>   	if (checksum != (u16)NVM_SUM) {
>>   		e_dbg("NVM Checksum Invalid\n");
>>   		return -E1000_ERR_NVM;
> 

No, it doesn't.

You are comparing the wrong value with NVM_SUM_FACTORY_DEFAULT. You 
should check it against the checksum word 0x3F (NVM bytes 0x7E and 0x7F) 
which is used to ensure that the base NVM image
is a valid image, and which in my case is left unchanged by Dell in the 
firmware.

I believe the changes should look something like this:

---
  drivers/net/ethernet/intel/e1000e/defines.h |  3 +++
  drivers/net/ethernet/intel/e1000e/nvm.c     | 13 ++++++++++++-
  2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h 
b/drivers/net/ethernet/intel/e1000e/defines.h
index 8294a7c4f122..996a0f4d2b49 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -638,6 +638,9 @@
  /* For checksumming, the sum of all words in the NVM should equal 
0xBABA. */
  #define NVM_SUM                    0xBABA

+/* Factory default value for the NVM checksum word */
+#define NVM_CHECKSUM_WORD_FACTORY_DEFAULT       0xFFFF
+
  /* PBA (printed board assembly) number words */
  #define NVM_PBA_OFFSET_0           8
  #define NVM_PBA_OFFSET_1           9
diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c 
b/drivers/net/ethernet/intel/e1000e/nvm.c
index e609f4df86f4..4620efac0208 100644
--- a/drivers/net/ethernet/intel/e1000e/nvm.c
+++ b/drivers/net/ethernet/intel/e1000e/nvm.c
@@ -547,7 +547,18 @@ s32 e1000e_validate_nvm_checksum_generic(struct 
e1000_hw *hw)
  {
  	s32 ret_val;
  	u16 checksum = 0;
-	u16 i, nvm_data;
+	u16 i, nvm_data, checksum_word;
+
+	ret_val = e1000_read_nvm(hw, NVM_CHECKSUM_REG, 1, &checksum_word);
+	if (ret_val) {
+		e_dbg("NVM Read Error\n");
+		return ret_val;
+	}
+
+	if (hw->mac.type == e1000_pch_tgp && checksum_word == 
(u16)NVM_CHECKSUM_WORD_FACTORY_DEFAULT) {
+		e_dbg("Factory-default NVM Checksum word on TGP platform - ignoring\n");
+		return 0;
+	}

  	for (i = 0; i < (NVM_CHECKSUM_REG + 1); i++) {
  		ret_val = e1000_read_nvm(hw, i, 1, &nvm_data);
--