linux-kernel - Re: [PATCH v4] PCI/PM: Prevent runtime suspend before devices are fully initialized

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] [day] [month] [year] [list]

Message-ID: <aXJjNfqcZrXw1_h2@google.com>
Date: Thu, 22 Jan 2026 09:49:41 -0800
From: Brian Norris <briannorris@...omium.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Marek Szyprowski <m.szyprowski@...sung.com>,
	Bjorn Helgaas <helgaas@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Lukas Wunner <lukas@...ner.de>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-pm@...r.kernel.org,
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v4] PCI/PM: Prevent runtime suspend before devices are
 fully initialized

Hi Rafael,

Thanks for your thoughts!

On Sun, Jan 18, 2026 at 12:53:21PM +0100, Rafael J. Wysocki wrote:
> On Sat, Jan 17, 2026 at 2:19 AM Brian Norris <briannorris@...omium.org> wrote:
> > I suppose one way to "solve" that is (untested):
> >
> > --- a/drivers/pci/bus.c
> > +++ b/drivers/pci/bus.c
> > @@ -380,8 +380,12 @@ void pci_bus_add_device(struct pci_dev *dev)
> >                 put_device(&pdev->dev);
> >         }
> >
> > +       if (dev->dev.parent)
> > +               pm_runtime_get_sync(dev->dev.parent);
> >         pm_runtime_set_active(&dev->dev);
> >         pm_runtime_enable(&dev->dev);
> > +       if (dev->dev.parent)
> > +               pm_runtime_put(dev->dev.parent);
> >
> >         if (!dn || of_device_is_available(dn))
> >                 pci_dev_allow_binding(dev);
> >
> > Personally, I'm more inclined to go back to v1, since it prepares the
> > runtime PM status when the device is first discovered. That way, its
> > ancestors are still active, avoiding these sorts of problems. I'm
> > frankly not sure of all the reasons Rafael recommended I make the
> > v1->v3->v4 changes, and now that they cause problems, I'm inclined to
> > question them again.
> >
> > Rafael, do you have any thoughts?
> 
> Yeah.
> 
> Move back pm_runtime_set_active(&dev->dev) back to pm_runtime_init()
> because that would prevent the parent from suspending and keep
> pm_runtime_enable() here because that would prevent the device itself
> from suspending between pm_runtime_init() and this place.

I'll admit, I was a little fuzzy on the details of the first part of the
sentence here -- specifically, that an "active" (but still "disabled")
device will prevent suspend of its parent. I suppose I'm more familiar
with the typical "disabled and suspended" device, which essentially has
no effect on its parent.

Anyway, that's basically v3, so I rerolled a v5 that looks similar.

> And I would add comments in both places.

I tried to add a short comment to each. It's an art form to write
exactly the right size of comment to make everyone happy (people
complain about too much commenting, and then others complain about
non-obvious behaviors that could have used more comments), especially
when it comes to something as tricky as runtime PM. At least I tried...

Brian