| 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
| ||
|
Message-ID: <CAPDyKFpdixuO_ySig9dajFsv40J7d5ML67h6MwCkemrq3fft_Q@mail.gmail.com>
Date: Mon, 15 Dec 2025 16:48:39 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Ed Tsai <ed.tsai@...iatek.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
chun-hung.wu@...iatek.com, freddy.shin@...iatek.com,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH v1] PM: runtime: Do not clear needs_force_resume with
enabled runtime PM
On Mon, 15 Dec 2025 at 15:21, Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Monday, December 15, 2025 1:21:42 PM CET ed.tsai@...iatek.com wrote:
> > From: Ed Tsai <ed.tsai@...iatek.com>
> >
> > needs_force_resume is a bitfield member in struct dev_pm_info and must
> > be accessed under the same lock as its bitfield group.
> >
> > A real concurrent write was observed between needs_force_resume and
> > idle_notification; stacks below:
> >
> > write needs_force_resume:
> > pm_runtime_reinit+0x110/0x360
> > really_probe+0xe0/0x464
> > __driver_probe_device+0x9c/0x104
> > driver_probe_device+0x3c/0x1a8
> > __device_attach_driver+0x100/0x17c
> > bus_for_each_drv+0x10c/0x168
> > __device_attach_async_helper+0x7c/0xf4
> > async_run_entry_fn+0x4c/0x1b4
> > process_scheduled_works+0x1dc/0x498
> > worker_thread+0x220/0x320
> > kthread+0x150/0x27c
> > ret_from_fork+0x10/0x20
> >
> > write idle_notification:
> > rpm_idle+0x464/0x5f8
> > __pm_runtime_idle+0x7c/0x170
> > scsi_autopm_put_device+0x18/0x28
> > scsi_sysfs_add_sdev+0x1a0/0x1d8
> > scsi_probe_and_add_lun+0xbd8/0xcd0
> > __scsi_add_device+0xb8/0x11c
> > ufshcd_async_scan+0xb4/0x3a4
> > async_run_entry_fn+0x4c/0x1b4
> > process_scheduled_works+0x1dc/0x498
> > worker_thread+0x220/0x320
> > kthread+0x150/0x27c
> > ret_from_fork+0x10/0x20
> >
> > Fixes: 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit()")
> > Signed-off-by: Ed Tsai <ed.tsai@...iatek.com>
> > ---
> > drivers/base/power/runtime.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> > index 84676cc24221..3328543b1ed8 100644
> > --- a/drivers/base/power/runtime.c
> > +++ b/drivers/base/power/runtime.c
> > @@ -1879,11 +1879,22 @@ void pm_runtime_reinit(struct device *dev)
> > pm_runtime_put(dev->parent);
> > }
> > }
> > +
> > /*
> > * Clear power.needs_force_resume in case it has been set by
> > * pm_runtime_force_suspend() invoked from a driver remove callback.
> > */
> > + if (dev->power.irq_safe)
> > + spin_lock(&dev->power.lock);
> > + else
> > + spin_lock_irq(&dev->power.lock);
> > +
> > dev->power.needs_force_resume = false;
> > +
> > + if (dev->power.irq_safe)
> > + spin_unlock(&dev->power.lock);
> > + else
> > + spin_unlock_irq(&dev->power.lock);
> > }
> >
> > /**
> >
>
> Thanks for the patch, but this happens because the flag is cleared when
> runtime PM is enabled which shouldn't be necessary, so I'd prefer to make
> the change below.
>
> ---
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Subject: [PATCH v1] PM: runtime: Do not clear needs_force_resume with enabled runtime PM
>
> Commit 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in
> pm_runtime_reinit()") added provisional clearing of power.needs_force_resume
> to pm_runtime_reinit(), but it is done unconditionally which is a
> mistake because pm_runtime_reinit() may race with driver probing
> and removal [1].
>
> To address this, notice that power.needs_force_resume should never
> be set when runtime PM is enabled and so it only needs to be cleared
> when runtime PM is disabled, and update pm_runtime_init() to only
> clear that flag when runtime PM is disabled.
>
> Fixes: 89d9cec3b1e9 ("PM: runtime: Clear power.needs_force_resume in pm_runtime_reinit()")
> Reported-by: Ed Tsai <ed.tsai@...iatek.com>
> Closes: https://lore.kernel.org/linux-pm/20251215122154.3180001-1-ed.tsai@mediatek.com/ [1]
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
Kind regards
Uffe
> ---
> drivers/base/power/runtime.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -1878,16 +1878,18 @@ void pm_runtime_init(struct device *dev)
> */
> void pm_runtime_reinit(struct device *dev)
> {
> - if (!pm_runtime_enabled(dev)) {
> - if (dev->power.runtime_status == RPM_ACTIVE)
> - pm_runtime_set_suspended(dev);
> - if (dev->power.irq_safe) {
> - spin_lock_irq(&dev->power.lock);
> - dev->power.irq_safe = 0;
> - spin_unlock_irq(&dev->power.lock);
> - if (dev->parent)
> - pm_runtime_put(dev->parent);
> - }
> + if (pm_runtime_enabled(dev))
> + return;
> +
> + if (dev->power.runtime_status == RPM_ACTIVE)
> + pm_runtime_set_suspended(dev);
> +
> + if (dev->power.irq_safe) {
> + spin_lock_irq(&dev->power.lock);
> + dev->power.irq_safe = 0;
> + spin_unlock_irq(&dev->power.lock);
> + if (dev->parent)
> + pm_runtime_put(dev->parent);
> }
> /*
> * Clear power.needs_force_resume in case it has been set by
>
>
>
Powered by blists - more mailing lists