linux-kernel - Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake system

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: <584af55f-1b73-4c17-bf85-c2d3ecf6692e@kernel.org>
Date: Fri, 27 Jun 2025 21:38:06 +0200
From: Hans de Goede <hansg@...nel.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>,
 Mario Limonciello <superm1@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
 Mika Westerberg <westeri@...nel.org>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, "open list:GPIO ACPI SUPPORT" <linux-gpio@...r.kernel.org>,
 "open list:GPIO ACPI SUPPORT" <linux-acpi@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>,
 "open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)..."
 <linux-input@...r.kernel.org>, Mario Limonciello <mario.limonciello@....com>
Subject: Re: [PATCH v3 4/4] Input: Don't send fake button presses to wake
 system

Hi,

On 27-Jun-25 9:18 PM, Dmitry Torokhov wrote:
> On Fri, Jun 27, 2025 at 01:56:53PM -0500, Mario Limonciello wrote:
>> On 6/27/2025 1:36 PM, Dmitry Torokhov wrote:
>>> On Fri, Jun 27, 2025 at 05:56:05PM +0200, Hans de Goede wrote:
> 
> [ ... trim ... ]
> 
>>>
>>> 2. There is a patch from Mario (a8605b0ed187) suppressing sending
>>> KEY_POWER as part of "normal" wakeup handling, pretty much the same as
>>> what he and you are proposing to do in gpio-keys (and eventually in
>>> every driver keyboard or button driver in the kernel). This means we no
>>> longer can tell if wakeup is done by power button or sleep button (on
>>> systems with dual-button models, see ACPI 4.8.3.1).
>>
>> Actually a8605b0ed187 was about a runtime regression not a suspend
>> regression.  I didn't change anything with sending KEY_POWER during wakeup
>> handling.
> 
> Ah, right, ignorng events for "suspended" buttons was done in
> e71eeb2a6bcc ("ACPI / button: Do not propagate wakeup-from-suspend
> events"). Again trying to add heuristic to the kernel instead of
> enlightening userspace.
> 
> I am curious why the system is sending "Notify Wake" events when not
> sleeping though?
> 
> [ .. skip .. ]
> 
>>
>> FTR I did test Hans suggestion and it does work effectively (code below).
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c
>> b/drivers/input/keyboard/gpio_keys.c
>> index f9db86da0818b..3bc8c95e9943b 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -425,7 +425,8 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void
>> *dev_id)
>>                          * already released by the time we got interrupt
>>                          * handler to run.
>>                          */
>> -                       input_report_key(bdata->input, button->code, 1);
>> +                       input_report_key(bdata->input, *bdata->code, 1);
>> +                       input_sync(bdata->input);
> 
> I start wondering if we should keep the fake press given that we do not
> know for sure if wakeup truly happened because of this button press...

AFAIK we cannot drop the fake press because then Android userspace
will immediately go back to sleep again assuming the wakeup was
e.g. just some data coming in from the modem which did not result
in a notification to show, so no need to turn on the display,
but instead immediately go back to sleep.

IIRC last time we had this discussion (man years ago) the reason
to send KEY_POWER was to let Android know that it should actualy
turn on the display and show the unlock screen because the user
wants that to happen.

I believe this is also what the KEY_WAKEUP thing in the ACPI button
code is for.

> Can we track back to the wakeup source and determine this? It will not
> help your problem, but I still believe userspace is where policy should
> live.

There is /sys/power/pm_wakeup_irq we could correlate that to the IRQ
number of the ISR and then AFAICT we will definitively know if
the power-button was the wakeup source ?

Regards,

Hans