linux-kernel - Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed during hibernation

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: <CAJZ5v0iWCxXSw3fBbesoMEWqGrRL9xrD85pMoW8rPuBBwTayhw@mail.gmail.com>
Date: Mon, 24 Nov 2025 19:42:40 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Muhammad Usama Anjum <usama.anjum@...labora.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>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Peter Zijlstra <peterz@...radead.org>, linux-acpi@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	linux-input@...r.kernel.org, kernel@...labora.com, superm1@...nel.org
Subject: Re: [PATCH 2/4] ACPI: button: Cancel hibernation if button is pressed
 during hibernation

On Fri, Nov 7, 2025 at 7:45 PM Muhammad Usama Anjum
<usama.anjum@...labora.com> wrote:
>
> acpi_pm_wakeup_event() is called from acpi_button_notify() which is
> called when power button is pressed. The system is worken up from s2idle
> in this case by setting hard parameter to pm_wakeup_dev_event().

Right, so presumably you want to set it for hibernation too.

> Call acpi_pm_wakeup_event() if power button is pressed and hibernation
> is in progress.

Well, this is not what the code does after the change.

> Set the hard parameter such that pm_system_wakeup()
> gets called which increments pm_abort_suspend counter. The explicit call
> to acpi_pm_wakeup_event() is necessary as ACPI button device has the
> wakeup source. Hence call to input_report_key() with input device
> doesn't call pm_system_wakeup() as it doesn't have wakeup source
> registered.
>
> Hence hibernation would be cancelled as in hibernation path, this counter
> is checked if it should be aborted.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> ---
> Changes since RFC:
> - Use pm_sleep_transition_in_progress()
> - Update descriptin why explicit call to acpi_pm_wakeup_event() is
>   necessary
> ---
>  drivers/acpi/button.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c
> index 3c6dd9b4ba0ad..e4be5c763edaf 100644
> --- a/drivers/acpi/button.c
> +++ b/drivers/acpi/button.c
> @@ -20,6 +20,7 @@
>  #include <linux/acpi.h>
>  #include <linux/dmi.h>
>  #include <acpi/button.h>
> +#include <linux/suspend.h>
>
>  #define ACPI_BUTTON_CLASS              "button"
>  #define ACPI_BUTTON_FILE_STATE         "state"
> @@ -458,11 +459,16 @@ static void acpi_button_notify(acpi_handle handle, u32 event, void *data)
>         acpi_pm_wakeup_event(&device->dev);

The above is what you want to change, as this already reports the
event.  Reporting it twice is unnecessary and potentially confusing.

>         button = acpi_driver_data(device);
> -       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
> -               return;
> -
>         input = button->input;
>         keycode = test_bit(KEY_SLEEP, input->keybit) ? KEY_SLEEP : KEY_POWER;
> +       if (event == ACPI_BUTTON_NOTIFY_STATUS && keycode == KEY_POWER &&
> +           pm_sleep_transition_in_progress()) {
> +               pm_wakeup_dev_event(&device->dev, 0, true);
> +               return;
> +       }

First, this will affect suspend too.

Second, this reports an already reported wakeup event.

Next, why KEY_POWER only?  Is KEY_SLEEP not expected to wake up?

And why event == ACPI_BUTTON_NOTIFY_STATUS?  Isn't this what
ACPI_BUTTON_NOTIFY_WAKE is for?

> +
> +       if (button->suspended || event == ACPI_BUTTON_NOTIFY_WAKE)
> +               return;
>
>         input_report_key(input, keycode, 1);
>         input_sync(input);
> --