linux-kernel - Re: [PATCH v1] PM: sleep: Update power.completion for all devices on errors

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: <CAPDyKFqs4-TZy8Vj72TwRUjoUp9V=yXGDUksRe7FhKVVwnvAuw@mail.gmail.com>
Date: Tue, 15 Jul 2025 13:38:55 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Tudor Ambarus <tudor.ambarus@...aro.org>, Saravana Kannan <saravanak@...gle.com>
Subject: Re: [PATCH v1] PM: sleep: Update power.completion for all devices on errors

On Mon, 14 Jul 2025 at 19:45, Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> After commit aa7a9275ab81 ("PM: sleep: Suspend async parents after
> suspending children"), the following scenario is possible:
>
>  1. Device A is async and it depends on device B that is sync.
>  2. Async suspend is scheduled for A before the processing of B is
>     started.
>  3. A is waiting for B.
>  4. In the meantime, an unrelated device fails to suspend and returns
>     an error.
>  5. The processing of B doesn't start at all and its power.completion is
>     not updated.
>  6. A is still waiting for B when async_synchronize_full() is called.
>  7. Deadlock ensues.
>
> To prevent this from happening, update power.completion for all devices
> on errors in all suspend phases, but do not do it directly for devices
> that are already being processed or are waiting for the processing to
> start because in those cases it may be necessary to wait for the
> processing to actually complete before updating power.completion for
> the device.
>
> Fixes: aa7a9275ab81 ("PM: sleep: Suspend async parents after suspending children")
> Fixes: 443046d1ad66 ("PM: sleep: Make suspend of devices more asynchronous")
> Closes: https://lore.kernel.org/linux-pm/e13740a0-88f3-4a6f-920f-15805071a7d6@linaro.org/
> Reported-and-tested-by: Tudor Ambarus <tudor.ambarus@...aro.org>
> 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/main.c |   19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1323,6 +1323,22 @@
>         device_links_read_unlock(idx);
>  }
>
> +static void dpm_async_suspend_complete_all(struct list_head *device_list)
> +{
> +       struct device *dev;
> +
> +       guard(mutex)(&async_wip_mtx);
> +
> +       list_for_each_entry_reverse(dev, device_list, power.entry) {
> +               /*
> +                * In case the device is being waited for and async processing
> +                * has not started for it yet, let the waiters make progress.
> +                */
> +               if (!dev->power.work_in_progress)
> +                       complete_all(&dev->power.completion);
> +       }
> +}
> +
>  /**
>   * resume_event - Return a "resume" message for given "suspend" sleep state.
>   * @sleep_state: PM message representing a sleep state.
> @@ -1499,6 +1515,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_late_early_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
> @@ -1701,6 +1718,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_suspended_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
> @@ -1994,6 +2012,7 @@
>                 mutex_lock(&dpm_list_mtx);
>
>                 if (error || async_error) {
> +                       dpm_async_suspend_complete_all(&dpm_prepared_list);
>                         /*
>                          * Move all devices to the target list to resume them
>                          * properly.
>
>
>