netdev - Re: [PATCH v3 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: <0407b67d-e63f-4a85-b3b4-1563335607dc@jacekk.info>
Date: Tue, 24 Jun 2025 23:05:05 +0200
From: Jacek Kowalski <jacek@...ekk.info>
To: Simon Horman <horms@...nel.org>
Cc: 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>,
 intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, Vlad URSU <vlad@...u.me>
Subject: Re: [PATCH v3 2/2] e1000e: ignore factory-default checksum value on
 TGP platform

>> Unfortunately some systems have left the factory with an empty
>> checksum. NVM is not modifiable on this platform, hence ignore
>> checksum 0xFFFF on Tiger Lake systems to work around this.
> 
> I think that you need to update the patch description. As of v3 it's
> the last word of the checksum that is being checked, not the entire
> checksum.

As I understood it, "sum" is the resulting value while "checksum" is the
value appended so that the "sum" is equal to some constant.

But my understanding is utterly broken by this line:

>> if (checksum != (u16)NVM_SUM) {

Where variable checksum (shall it be "sum"?) that includes
"checksum" (or maybe checksum word?) from the *checksum* register 
address (NVM_CHECKSUM_REG) is compared to a constant called "NVM_SUM".

Is something like this fine by you:

> Unfortunately some systems have left the factory with an unmodified
> value of 0xFFFF at register address 0x3F (checksum word location).
> So on Tiger Lake platform we ignore the computed checksum when such
> condition is encountered.

?

>> +#define NVM_CHECKSUM_FACTORY_DEFAULT 0xFFFF
> 
> Perhaps it is too long, but I liked Vlad's suggestion of naming this
> NVM_CHECKSUM_WORD_FACTORY_DEFAULT.

I can update it as well once we agree on the wording.

>> +	if (hw->mac.type == e1000_pch_tgp && nvm_data == NVM_CHECKSUM_FACTORY_DEFAULT) {
> 
> Please wrap the line above so it is 80 columns wide or less.

Wilco.

-- 
Best regards,
   Jacek Kowalski