[PATCH] xen/manage: Fix suspend error path

Lukas Wunner posted 1 patch 1 week, 2 days ago
Failed in applying to current master (apply log)
drivers/xen/manage.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] xen/manage: Fix suspend error path
Posted by Lukas Wunner 1 week, 2 days ago
The device power management API has the following asymmetry:
* dpm_suspend_start() does not clean up on failure
  (it requires a call to dpm_resume_end())
* dpm_suspend_end() does clean up on failure
  (it does not require a call to dpm_resume_start())

The asymmetry was introduced by commit d8f3de0d2412 ("Suspend-related
patches for 2.6.27") in June 2008:  It removed a call to device_resume()
from device_suspend() (which was later renamed to dpm_suspend_start()).

When Xen began using the device power management API in May 2008 with
commit 0e91398f2a5d ("xen: implement save/restore"), the asymmetry did
not yet exist.  But since it was introduced, a call to dpm_resume_end()
is missing in the error path of dpm_suspend_start().  Fix it.

Fixes: d8f3de0d2412 ("Suspend-related patches for 2.6.27")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org  # v2.6.27
---
kexec suffered from the same issue ever since it began using the
device power management API in July 2008 with commit 89081d17f7bb
("kexec jump: save/restore device state").  It was fixed this year
by commit 996afb6efd1a ("kexec_core: Fix error code path in the
KEXEC_JUMP flow").  All other callers seem fine.

 drivers/xen/manage.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 841afa4..1f5a7a4 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -110,7 +110,7 @@ static void do_suspend(void)
 	err = dpm_suspend_start(PMSG_FREEZE);
 	if (err) {
 		pr_err("%s: dpm_suspend_start %d\n", __func__, err);
-		goto out_thaw;
+		goto out_resume_end;
 	}
 
 	printk(KERN_DEBUG "suspending xenstore...\n");
@@ -150,6 +150,7 @@ static void do_suspend(void)
 	else
 		xs_suspend_cancel();
 
+out_resume_end:
 	dpm_resume_end(si.cancelled ? PMSG_THAW : PMSG_RESTORE);
 
 out_thaw:
-- 
2.50.1
Re: [PATCH] xen/manage: Fix suspend error path
Posted by Rafael J. Wysocki 1 week, 2 days ago
On Thu, Sep 4, 2025 at 3:11 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> The device power management API has the following asymmetry:
> * dpm_suspend_start() does not clean up on failure
>   (it requires a call to dpm_resume_end())
> * dpm_suspend_end() does clean up on failure
>   (it does not require a call to dpm_resume_start())
>
> The asymmetry was introduced by commit d8f3de0d2412 ("Suspend-related
> patches for 2.6.27") in June 2008:  It removed a call to device_resume()
> from device_suspend() (which was later renamed to dpm_suspend_start()).
>
> When Xen began using the device power management API in May 2008 with
> commit 0e91398f2a5d ("xen: implement save/restore"), the asymmetry did
> not yet exist.  But since it was introduced, a call to dpm_resume_end()
> is missing in the error path of dpm_suspend_start().  Fix it.
>
> Fixes: d8f3de0d2412 ("Suspend-related patches for 2.6.27")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org  # v2.6.27

Thanks for catching this!

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>

> ---
> kexec suffered from the same issue ever since it began using the
> device power management API in July 2008 with commit 89081d17f7bb
> ("kexec jump: save/restore device state").  It was fixed this year
> by commit 996afb6efd1a ("kexec_core: Fix error code path in the
> KEXEC_JUMP flow").  All other callers seem fine.
>
>  drivers/xen/manage.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 841afa4..1f5a7a4 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -110,7 +110,7 @@ static void do_suspend(void)
>         err = dpm_suspend_start(PMSG_FREEZE);
>         if (err) {
>                 pr_err("%s: dpm_suspend_start %d\n", __func__, err);
> -               goto out_thaw;
> +               goto out_resume_end;
>         }
>
>         printk(KERN_DEBUG "suspending xenstore...\n");
> @@ -150,6 +150,7 @@ static void do_suspend(void)
>         else
>                 xs_suspend_cancel();
>
> +out_resume_end:
>         dpm_resume_end(si.cancelled ? PMSG_THAW : PMSG_RESTORE);
>
>  out_thaw:
> --
> 2.50.1
>
>