linux-kernel - Re: [QUESTION] pci_read_bridge_bases: skip prefetch window if pref

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: <aWdG/9N2C/7L5sFQ@DESKTOP-TIT0J8O.localdomain>
Date: Wed, 14 Jan 2026 11:34:23 +0400
From: Ahmed Naseef <naseefkm@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [QUESTION] pci_read_bridge_bases: skip prefetch window if
 pref_window not set?

On Tue, Jan 13, 2026 at 03:02:59PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 12, 2026 at 01:41:00PM +0400, Ahmed Naseef wrote:
> >   [  160.238227] mtk-pcie 1fb83000.pcie: EN7528: port1 link trained to Gen2
> 
> Can we look forward to a patch to add support for EN7528?

Definitely. Someone else is working on PCIe support for EN751221
(same SoC family). Once everything is consolidated, we plan to submit
a unified patch to drivers/pci/controller/pcie-mediatek.c.

> > PCI_PREF_MEMORY_LIMIT which both return 0x0000. This results in base = 0
> > and limit = 0. The condition "if (base <= limit)" evaluates to true
> > (since 0 <= 0), so a bogus prefetch window [mem 0x00000000-0x000fffff pref]
> > is created.
> 
> It's too bad we didn't log this in dmesg.  It looks like we claimed
> there was a [mem 0x28100000-0x282fffff pref] window.

Should I add logging for this as part of the patch? If so, could you
suggest where and what format would be appropriate?
> 
> > 
> >         pci_read_bridge_io(child->self, child->resource[0], false);
> >         pci_read_bridge_mmio(child->self, child->resource[1], false);
> > -       pci_read_bridge_mmio_pref(child->self, child->resource[2], false);
> > +       if (dev->pref_window)
> > +               pci_read_bridge_mmio_pref(child->self, child->resource[2], false);
> 
> This seems reasonable to me, but I wonder if we should put the test
> inside pci_read_bridge_mmio_pref() instead of relying on the caller to
> do the test?
> 
> Also, I suspect the IO window has a similar problem since it is also
> optional.  What do you think?
> 

You're right on both points. Updated patch below with checks inside
both functions:


---
From: Ahmed Naseef <naseefkm@...il.com>
Subject: [PATCH] PCI: Skip bridge window reads when window is not supported

pci_read_bridge_io() and pci_read_bridge_mmio_pref() read bridge window
registers unconditionally. If the registers are hardwired to zero
(not implemented), both base and limit will be 0. Since (0 <= 0) is
true, a bogus window [mem 0x00000000-0x000fffff] or [io 0x0000-0x0fff]
gets created.

pci_read_bridge_windows() already detects unsupported windows by
testing register writability and sets io_window/pref_window flags
accordingly. Check these flags at the start of pci_read_bridge_io()
and pci_read_bridge_mmio_pref() to skip reading registers when the
window is not supported.

Suggested-by: Bjorn Helgaas <helgaas@...nel.org>
Signed-off-by: Ahmed Naseef <naseefkm@...il.com>
---

--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -351,6 +351,9 @@ static void pci_read_bridge_io(struct pc
        unsigned long io_mask, io_granularity, base, limit;
        struct pci_bus_region region;

+       if (!dev->io_window)
+               return;
+
        io_mask = PCI_IO_RANGE_MASK;
        io_granularity = 0x1000;
        if (dev->io_window_1k) {
@@ -412,6 +415,9 @@ static void pci_read_bridge_mmio_pref(st
        pci_bus_addr_t base, limit;
        struct pci_bus_region region;

+       if (!dev->pref_window)
+               return;
+
        pci_read_config_word(dev, PCI_PREF_MEMORY_BASE, &mem_base_lo);
        pci_read_config_word(dev, PCI_PREF_MEMORY_LIMIT, &mem_limit_lo);
        base64 = (mem_base_lo & PCI_PREF_RANGE_MASK) << 16;
		
		
Once you confirm, I will send this as a fresh patch.

Best regards,
Ahmed Naseef