linux-kernel - RE: [PATCH 1/1] pci: Add subordinate check before pci_add_new

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <DS4PPFD24E991EC6BCD98A674667D0D7AB39634A@DS4PPFD24E991EC.namprd11.prod.outlook.com>
Date: Fri, 15 Aug 2025 02:31:31 +0000
From: "He, Rui" <Rui.He@...driver.com>
To: Bjorn Helgaas <helgaas@...nel.org>
CC: Bjorn Helgaas <bhelgaas@...gle.com>,
        "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "Chikhalkar, Prashant"
	<Prashant.Chikhalkar@...driver.com>,
        "Xiao, Jiguang"
	<Jiguang.Xiao@...driver.com>
Subject: RE: [PATCH 1/1] pci: Add subordinate check before pci_add_new_bus()



> -----Original Message-----
> From: Bjorn Helgaas <helgaas@...nel.org>
> Sent: 2025年8月15日 4:36
> To: He, Rui <Rui.He@...driver.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>; linux-pci@...r.kernel.org;
> linux-kernel@...r.kernel.org; Chikhalkar, Prashant
> <Prashant.Chikhalkar@...driver.com>; Xiao, Jiguang
> <Jiguang.Xiao@...driver.com>
> Subject: Re: [PATCH 1/1] pci: Add subordinate check before
> pci_add_new_bus()
> 
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe.
> 
> On Thu, Aug 14, 2025 at 05:39:37PM +0800, Rui He wrote:
> > For preconfigured PCI bridge, child bus created on the first scan.
> > While for some reasons(e.g register mutation), the secondary, and
> > subordiante register reset to 0 on the second scan, which caused to
> > create PCI bus twice for the same PCI device.
> 
> I don't quite follow this.  Do you mean something is changing the bridge
> configuration between the first and second scans?

I'm not sure what changed the bridge configuration, but the secondary and 
subordinate is indeed 0 on the second scan as [bus 0e-10] created for 0000:0b:01.0.

In my opinion, it might be an invalid communication or register mutation in PCI bridge.

> 
> > Following is the related log:
> > [Wed May 28 20:38:36 CST 2025] pci 0000:0b:01.0: PCI bridge to [bus
> > 0d] [Wed May 28 20:38:36 CST 2025] pci 0000:0b:05.0: bridge
> > configuration invalid ([bus 00-00]), reconfiguring [Wed May 28
> > 20:38:36 CST 2025] pci 0000:0b:01.0: PCI bridge to [bus 0e-10] [Wed
> > May 28 20:38:36 CST 2025] pci 0000:0b:05.0: PCI bridge to [bus 0f-10]
> 
> Drop the timestamps (since they don't contribute to understanding the
> problem) and indent the logs a couple spaces.
> 

OK

> > Here PCI device 000:0b:01.0 assigend to bus 0d and 0e.
> 
> It looks like the [bus 0f-10] range is assigned to both bridges
> (0b:01.0 and 0b:05.0), which would definitely be a problem.
> 
> I'm surprised that we haven't tripped over this before, and I'm curious about
> how we got here.  Can you set CONFIG_DYNAMIC_DEBUG=y, boot with the
> dyndbg="file drivers/pci/* +p" kernel parameter, and collect the complete
> dmesg log?
>

Sorry, as this is a individual issue, and cannot be reproduced, I cannot offer more detailed logs.

> > This patch checks if child PCI bus has been created on the second scan
> > of bridge. If yes, return directly instead of create a new one.
> >
> > Signed-off-by: Rui He <rui.he@...driver.com>
> > ---
> >  drivers/pci/probe.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > f41128f91ca76..ec67adbf31738 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1444,6 +1444,9 @@ static int pci_scan_bridge_extend(struct pci_bus
> *bus, struct pci_dev *dev,
> >                       goto out;
> >               }
> >
> > +             if(pci_has_subordinate(dev))
> > +                     goto out;
> 
> Follow the coding style, i.e., add a space in "if (pci_..."

Will be changed in v2.

> 
> >               /* Clear errors */
> >               pci_write_config_word(dev, PCI_STATUS, 0xffff);
> >
> > --
> > 2.43.0
> >