linux-kernel - Re: [PATCH v2 2/3] PCI: Fix pdev_resources

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: <b873e7dc-8cbe-3370-4b47-8c1cb2e6da6e@linux.intel.com>
Date: Thu, 21 Aug 2025 18:21:22 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Bjorn Helgaas <helgaas@...nel.org>
cc: linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    Tudor Ambarus <tudor.ambarus@...aro.org>, Rio <rio@....me>, 
    D Scott Phillips <scott@...amperecomputing.com>, 
    LKML <linux-kernel@...r.kernel.org>, 
    Christian König <christian.koenig@....com>, 
    stable@...r.kernel.org
Subject: Re: [PATCH v2 2/3] PCI: Fix pdev_resources_assignable() disparity

On Thu, 21 Aug 2025, Bjorn Helgaas wrote:

> On Mon, Jun 30, 2025 at 05:26:40PM +0300, Ilpo Järvinen wrote:
> > pdev_sort_resources() uses pdev_resources_assignable() helper to decide
> > if device's resources cannot be assigned. pbus_size_mem(), on the other
> > hand, does not do the same check. This could lead into a situation
> > where a resource ends up on realloc_head list but is not on the head
> > list, which is turn prevents emptying the resource from the
> > realloc_head list in __assign_resources_sorted().
> > 
> > A non-empty realloc_head is unacceptable because it triggers an
> > internal sanity check as show in this log with a device that has class
> > 0 (PCI_CLASS_NOT_DEFINED):
> 
> Is the class relevant here?

It actually is, because pdev_resources_assignable() checks for it. In case 
of this particular device there was 0 there causing leading eventually to 
this internal sanity check problem.

> > pci 0001:01:00.0: [144d:a5a5] type 00 class 0x000000 PCIe Endpoint
> > pci 0001:01:00.0: BAR 0 [mem 0x00000000-0x000fffff 64bit]
> > pci 0001:01:00.0: ROM [mem 0x00000000-0x0000ffff pref]
> > pci 0001:01:00.0: enabling Extended Tags
> > pci 0001:01:00.0: PME# supported from D0 D3hot D3cold
> > pci 0001:01:00.0: 15.752 Gb/s available PCIe bandwidth, limited by 8.0 GT/s PCIe x2 link at 0001:00:00.0 (capable of 31.506 Gb/s with 16.0 GT/s PCIe x2 link)
> > pcieport 0001:00:00.0: bridge window [mem 0x00100000-0x001fffff] to [bus 01-ff] add_size 100000 add_align 100000
> > pcieport 0001:00:00.0: bridge window [mem 0x40000000-0x401fffff]: assigned
> > ------------[ cut here ]------------
> > kernel BUG at drivers/pci/setup-bus.c:2532!
> > Internal error: Oops - BUG: 00000000f2000800 [#1]  SMP
> > ...
> > Call trace:
> >  pci_assign_unassigned_bus_resources+0x110/0x114 (P)
> >  pci_rescan_bus+0x28/0x48
> > 
> > Use pdev_resources_assignable() also within pbus_size_mem() to skip
> > processing of non-assignable resources which removes the disparity in
> > between what resources pdev_sort_resources() and pbus_size_mem()
> > consider. As non-assignable resources are no longer processed, they are
> > not added to the realloc_head list, thus the sanity check no longer
> > triggers.
> > 
> > This disparity problem is very old but only now became apparent after
> > the commit 2499f5348431 ("PCI: Rework optional resource handling") that
> > made the ROM resources optional when calculating bridge window sizes
> > which required adding the resource to the realloc_head list.
> > Previously, bridge windows were just sized larger than necessary.
> > 
> > Fixes: 2499f5348431 ("PCI: Rework optional resource handling")
> > Reported-by: Tudor Ambarus <tudor.ambarus@...aro.org>
> 
> Any URL we can include here for the report?

This was discussed in the thread of the original patch/series:

Link: https://lore.kernel.org/all/5f103643-5e1c-43c6-b8fe-9617d3b5447c@linaro.org/

> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> > Cc: <stable@...r.kernel.org>
> > ---
> >  drivers/pci/setup-bus.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index f90d49cd07da..24863d8d0053 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -1191,6 +1191,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> >  			resource_size_t r_size;
> >  
> >  			if (r->parent || (r->flags & IORESOURCE_PCI_FIXED) ||
> > +			    !pdev_resources_assignable(dev) ||
> >  			    ((r->flags & mask) != type &&
> >  			     (r->flags & mask) != type2 &&
> >  			     (r->flags & mask) != type3))
> > -- 
> > 2.39.5
> > 
> 

-- 
 i.