Thread (1 message) 1 message, 1 author, 3d ago

Re: [PATCH net v3] e1000e: Fix out-of-bounds MMIO access by validating BAR0 size

From: Pu Lehui <hidden>
Date: 2026-07-24 01:27:49
Also in: sashiko-reviews

Gentle ping~ hope this message would not miss.

And +cc netdev@vger.kernel.org as I missed it.


On 2026/7/22 16:34, Pu Lehui wrote:
On 2026/7/22 11:37, sashiko-bot@kernel.org wrote:
quoted
Thank you for your contribution! Sashiko AI review found 1 potential 
issue(s) to consider:

Pre-existing issues:
- [High] The newly introduced minimum BAR0 length check 
`E1000_MMIO_LEN_MIN` is too small because it fails to account for 
flash registers mapped into the GbE memory space for newer MACs, 
leaving the driver vulnerable to the same OOB MMIO access panics it 
attempts to fix.
-- 
--- Patch [1]: [PATCH net v3] e1000e: Fix out-of-bounds MMIO access by 
validating BAR0 size ---
commit 9c24a014d2bfbd3a6822ff92be6a5c7aceb588d4
Author: Pu Lehui [off-list ref]
Subject: e1000e: Fix out-of-bounds MMIO access by validating BAR0 size

This patch adds a check during e1000_probe() to validate that the 
device's
BAR0 size is large enough to cover the highest register offset 
(E1000_SYSSTMPH)
accessed by the driver. This is intended to fix a kernel panic caused by
out-of-bounds MMIO reads when dealing with small BAR0 allocations.
quoted
diff --git a/drivers/net/ethernet/intel/e1000e/regs.h 
b/drivers/net/ethernet/intel/e1000e/regs.h
index 6c0cd8cab3ef2..1e875227eef24 100644
--- a/drivers/net/ethernet/intel/e1000e/regs.h
+++ b/drivers/net/ethernet/intel/e1000e/regs.h
[ ... ]
quoted
@@ -242,4 +242,7 @@
  /* PHY registers */
  #define I82579_DFT_CTRL    PHY_REG(769, 20)
+/* Smallest BAR0 that covers every register the driver accesses */
+#define E1000_MMIO_LEN_MIN    (E1000_SYSSTMPH + sizeof(u32))
[Severity: High]
This isn't a bug introduced by this patch, but does this minimum length
cover all register accesses for newer MACs?

For MAC types e1000_pch_spt and newer, the driver accesses flash 
registers
mapped at E1000_FLASH_BASE_ADDR (0xE000). For example, during flash read:

drivers/net/ethernet/intel/e1000e/ich8lan.c:e1000_init_nvm_params_ich8lan() {
     ...
     hw->flash_address = hw->hw_addr + E1000_FLASH_BASE_ADDR;
     ...
}

And then later accesses like:

drivers/net/ethernet/intel/e1000e/ich8lan.c:e1000_read_flash_data32_ich8lan() {
     ...
     *data = er32flash(ICH_FLASH_FDATA0);
     ...
}

This results in an access at offset 0xE010 (57360 bytes).

Since E1000_MMIO_LEN_MIN is currently based on E1000_SYSSTMPH
(0xB64C + 4 = 46672 bytes), could a malicious or mocked PCI device 
expose a
BAR0 size of 12 pages (49152 bytes), pass this new check, and still 
trigger
an out-of-bounds MMIO read panic when accessing the flash space?
Hi Andrew,

Sashiko is right about this one.

On ich chipsets, the driver sets the flash base address like this:

hw->flash_address = hw->hw_addr + E1000_FLASH_BASE_ADDR; // 0xE000

Then it accesses flash registers up to ICH_FLASH_PR0 (0x0074), so the 
max offset ends up being 0xE000 + 0x0074 + sizeof(u32) — that’s about 
57.1 KB. Pretty subtle, and yeah, we totally missed it in v3.

Since PCI BAR sizes are power-of-two aligned, the smallest standard BAR 
size that can cover accesses beyond 57.1 KB is 64 KB.

So for v4, I’d like to set E1000_MMIO_LEN_MIN to SZ_64K. That keeps us 
safely covering the largest register offset we use, avoids fragile 
hardcoded values, and should be more future-proof if other chipset flash 
ranges come into play.

wdyt? :)

Thanks,
Lehui
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help