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: <CAJZ5v0i7zuAmvWGPjGKkMKV=qmpc6dJOQXRUVUoqtzd1bdeEdw@mail.gmail.com>
Date: Fri, 6 Feb 2026 20:40:42 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jacky Bai <ping.bai@....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, 
	driver-core@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	imx@...ts.linux.dev
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

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=pm&id=0491f3f9f664e7e0131eb4d2a8b19c49562e5c64

> ---
>  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..561e24c257d779db51a0f0d50dcfee61e98de64f 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>
>
>