netdev - Re: [Intel-wired-lan] [PATCH] e1000e: add option not to verify NVM checksum

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: <1c4b00b6-f6e3-4b04-a129-24452df60903@jacekk.info>
Date: Sat, 12 Apr 2025 01:14:17 +0200
From: Jacek Kowalski <jacek@...ekk.info>
To: "Lifshits, Vitaly" <vitaly.lifshits@...el.com>,
 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: [Intel-wired-lan] [PATCH] e1000e: add option not to verify NVM
 checksum

>> I'll experiment a little more and get back to you.>> Specifically I'll try to dump the NVM contents before
>> and after running e1000e_update_nvm_checksum and after
>> a reboot.

I finally had a moment to take a look at the issue again.

This change also makes everything work on my system:

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 364378133526..4538059091e6 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -4266,7 +4266,7 @@ static s32 e1000_validate_nvm_checksum_ich8lan(struct e1000_hw *hw)
         if (!(data & valid_csum_mask)) {
                 e_dbg("NVM Checksum valid bit not set\n");

-               if (hw->mac.type < e1000_pch_tgp) {
+               if (hw->mac.type <= e1000_pch_tgp) {
                         data |= valid_csum_mask;
                         ret_val = e1000_write_nvm(hw, word, 1, &data);
                         if (ret_val)

(the modification is not persisted - it is lost even after rmmod/insmod).

The diff between "before" and "after" NVM rewrite looks like this
(MAC address masked):

< 0x0000:               XX XX XX XX XX XX 00 08 ff ff 84 00 01 00 70 00
---
> 0x0000:               XX XX XX XX XX XX 01 08 ff ff 84 00 01 00 70 00 
10c10
< 0x0070:               ff ff ff ff ff ff ff ff ff ff 00 02 ff ff fe 36
---
> 0x0070:               ff ff ff ff ff ff ff ff ff ff 00 02 ff ff fd 34

Reading https://bugzilla.kernel.org/show_bug.cgi?id=213667 the issue
started with yet another Dell system, Precision 7760, locking itself
up with such modification.

The "fix" (4051f68318: e1000e: Do not take care about recovery NVM checksum)
fixed some problems (i.e. Precision 7760) and "broke" some configurations
(i.e. mine Latitude 5420).

The condition itself was changed once already (ffd24fa2fc: e1000e: Correct
NVM checksum verification flow).

> If this approach is acceptable to you, I will prepare a patch with
> the proposed fix and send it to you next week for testing on your system.

What solution do you have in mind?

The only one I can think of is to ignore the checksum completely if the
valid_csum_mask condition is not met on e1000_pch_tgp.

-- 
Best regards,
   Jacek Kowalski