linux-kernel - Re: [PATCH v2] PM: sleep: wakeirq: harden dev_pm_clear_wake

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: <CAJZ5v0jN9fU9NdWqc-+F5hiSEP4JkR=_qcdGzzHtk1i5tvCDbQ@mail.gmail.com>
Date: Tue, 3 Feb 2026 22:25:12 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Gui-Dong Han <hanguidong02@...il.com>
Cc: rafael@...nel.org, pavel@...nel.org, lenb@...nel.org, 
	gregkh@...uxfoundation.org, dakr@...nel.org, tony@...mide.com, 
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	baijiaju1990@...il.com
Subject: Re: [PATCH v2] PM: sleep: wakeirq: harden dev_pm_clear_wake_irq()
 against races

On Tue, Feb 3, 2026 at 4:20 AM Gui-Dong Han <hanguidong02@...il.com> wrote:
>
> dev_pm_clear_wake_irq() currently uses a dangerous pattern where
> dev->power.wakeirq is read and checked for NULL outside the lock. If two
> callers invoke this function concurrently, both might see a valid
> pointer and proceed. This could result in a double-free when the second
> caller acquires the lock and tries to release the same object.
>
> Address this by removing the lockless check of dev->power.wakeirq.
> Instead, acquire dev->power.lock immediately to ensure the check and the
> subsequent operations are atomic. If dev->power.wakeirq is NULL under
> the lock, simply unlock and return. This guarantees that concurrent
> calls cannot race to free the same object.
>
> Based on a quick scan of current users, I did not find an actual bug as
> drivers seem to rely on their own synchronization. However, since
> asynchronous usage patterns exist (e.g., in
> drivers/net/wireless/ti/wlcore), I believe a race is theoretically
> possible if the API is used less carefully in the future. This change
> hardens the API to be robust against such cases.
>
> Fixes: 4990d4fe327b ("PM / Wakeirq: Add automated device wake IRQ handling")
> Signed-off-by: Gui-Dong Han <hanguidong02@...il.com>

Patch applied as 6.20/7.0 material.

> ---
> @Rafael J. Wysocki: While studying wakeirq.c, I noticed comments for
> dev_pm_enable_wake_irq_check() and friends are outdated. They claim
> "Caller must hold &dev->power.lock" and limit usage to
> rpm_suspend/resume, yet pm_runtime_force_suspend/resume() call them
> lockless. Should I submit a follow-up patch to simply remove these
> restrictions or complicate the text with exceptions?

Please just send a separate patch to fix the outdated comments.

Thanks!

> v2:
> * Remove the lockless check and perform the check protected by the lock
> to avoid races, as suggested by Rafael J. Wysocki.
> v1:
> * Initial fix attempt using double-checked locking.
> ---
>  drivers/base/power/wakeirq.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/power/wakeirq.c b/drivers/base/power/wakeirq.c
> index 8aa28c08b289..c0809d18fc54 100644
> --- a/drivers/base/power/wakeirq.c
> +++ b/drivers/base/power/wakeirq.c
> @@ -83,13 +83,16 @@ EXPORT_SYMBOL_GPL(dev_pm_set_wake_irq);
>   */
>  void dev_pm_clear_wake_irq(struct device *dev)
>  {
> -       struct wake_irq *wirq = dev->power.wakeirq;
> +       struct wake_irq *wirq;
>         unsigned long flags;
>
> -       if (!wirq)
> +       spin_lock_irqsave(&dev->power.lock, flags);
> +       wirq = dev->power.wakeirq;
> +       if (!wirq) {
> +               spin_unlock_irqrestore(&dev->power.lock, flags);
>                 return;
> +       }
>
> -       spin_lock_irqsave(&dev->power.lock, flags);
>         device_wakeup_detach_irq(dev);
>         dev->power.wakeirq = NULL;
>         spin_unlock_irqrestore(&dev->power.lock, flags);
> --
> 2.43.0
>
>