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

Message-ID:
 <VE1PR04MB7213E7FE80A3D089291340C08767A@VE1PR04MB7213.eurprd04.prod.outlook.com>
Date: Sat, 7 Feb 2026 00:18:45 +0000
From: Jacky Bai <ping.bai@....com>
To: Frank Li <frank.li@....com>
CC: "Rafael J. Wysocki" <rafael@...nel.org>, 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 06, 2026 at 11:05:11PM +0800, Jacky Bai 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>
> > ---
> >  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);
> 
> Thanks, it is really hard to find this problem. I just worry about if it is enough.
> 
> for example in __dpm_async(), if "check power.work_in_progress" should be
> protect also.  And does other power.xx need be protect?
> 

For other bits, it seems no chance to trigger concurrent access with the work_in_progress bit. 

BR
Jacky Bai
> Frank
> 
> >
> >  	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>
> >