linux-kernel - RE: [PATCH] PM: sleep: core: Fix race condition in async noirq suspend

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:
 <VE1PR04MB721393217EA1792A7B8AE7DA8767A@VE1PR04MB7213.eurprd04.prod.outlook.com>
Date: Sat, 7 Feb 2026 00:16:24 +0000
From: Jacky Bai <ping.bai@....com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: Len Brown <lenb@...nel.org>, Pavel Machek <pavel@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Danilo Krummrich
	<dakr@...nel.org>, "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"driver-core@...ts.linux.dev" <driver-core@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>
Subject: RE: [PATCH] PM: sleep: core: Fix race condition in async noirq
 suspend

> Subject: Re: [PATCH] PM: sleep: core: Fix race condition in async noirq
> suspend
> 
> On Fri, Feb 6, 2026 at 4:03 PM Jacky Bai <ping.bai@....com> wrote:
> >
> > A race condition occurs during the noirq suspend stage when accessing
> > bitfields in struct dev_pm_info, causing system suspend to hang.
> >
> > During async noirq suspend, leaf devices are queued for asynchronous
> > processing while the main suspend thread continues clearing the
> > 'work_in_progress' flag of other devices, including potential parents
> > of those leaf devices.
> >
> > In struct dev_pm_info, 'work_in_progress' and 'must_resume' are
> > bitfields stored in the same byte. This creates a race when:
> >
> > 1. A leaf device's async thread sets its parent's 'must_resume' flag
> >    via dpm_superior_set_must_resume()
> > 2. Simultaneously, the main thread clears the parent's
> >    'work_in_progress' flag
> >
> > Since bitfield operations are not atomic, concurrent modifications to
> > the same byte can corrupt both fields. This leaves the parent device
> > in an incorrect state, causing the system suspend to hang.
> >
> > Fix this by adding lock protection around the bitfield accesses to
> > ensure atomic read-modify-write operations.
> >
> > Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more
> > asynchronous")
> > Signed-off-by: Jacky Bai <ping.bai@....com>
> 
> There is a fix for this already in my tree, see
> 
Thx Rafael. The patch in your tree provide a non-locking way. My patch can be dropped.
I Will pick this patch Into our internal tree to fix the issue we meet.

BR
Jacky Bai
> 
> > ---
> >  drivers/base/power/main.c | 20 +++++++++++++++++---
> >  1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index
> >
> 189de5250f256bdc1cdc33006b2c386d0794485f..561e24c257d779db51a0f0d
> 50dcf
> > ee61e98de64f 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -633,7 +633,9 @@ static bool __dpm_async(struct device *dev,
> async_func_t func)
> >         if (!is_async(dev))
> >                 return false;
> >
> > +       spin_lock_irq(&dev->power.lock);
> >         dev->power.work_in_progress = true;
> > +       spin_unlock_irq(&dev->power.lock);
> >
> >         get_device(dev);
> >
> > @@ -656,8 +658,11 @@ static int dpm_async_with_cleanup(struct device
> > *dev, void *fn)  {
> >         guard(mutex)(&async_wip_mtx);
> >
> > -       if (!__dpm_async(dev, fn))
> > +       if (!__dpm_async(dev, fn)) {
> > +               spin_lock_irq(&dev->power.lock);
> >                 dev->power.work_in_progress = false;
> > +               spin_unlock_irq(&dev->power.lock);
> > +       }
> >
> >         return 0;
> >  }
> > @@ -698,7 +703,10 @@ static void dpm_async_resume_subordinate(struct
> > device *dev, async_func_t func)  static void
> > dpm_clear_async_state(struct device *dev)  {
> >         reinit_completion(&dev->power.completion);
> > +
> > +       spin_lock_irq(&dev->power.lock);
> >         dev->power.work_in_progress = false;
> > +       spin_unlock_irq(&dev->power.lock);
> >  }
> >
> >  static bool dpm_root_device(struct device *dev) @@ -1407,13 +1415,19
> > @@ static void dpm_superior_set_must_resume(struct device *dev)
> >         struct device_link *link;
> >         int idx;
> >
> > -       if (dev->parent)
> > +       if (dev->parent) {
> > +               spin_lock_irq(&dev->parent->power.lock);
> >                 dev->parent->power.must_resume = true;
> > +               spin_unlock_irq(&dev->parent->power.lock);
> > +       }
> >
> >         idx = device_links_read_lock();
> >
> > -       dev_for_each_link_to_supplier(link, dev)
> > +       dev_for_each_link_to_supplier(link, dev) {
> > +               spin_lock_irq(&link->supplier->power.lock);
> >                 link->supplier->power.must_resume = true;
> > +               spin_unlock_irq(&link->supplier->power.lock);
> > +       }
> >
> >         device_links_read_unlock(idx);  }
> >
> > ---
> > base-commit: 9845cf73f7db6094c0d8419d6adb848028f4a921
> > change-id: 20260206-noirq_async_suspend-645bd6b8305e
> >
> > Best regards,
> > --
> > Jacky Bai <ping.bai@....com>
> >
> >