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: <75fixx6rgwsgsw6e765oxdcivcg2nkzx2fp2qywgx4vi3ihywh@ot7gdecsnttw>
Date: Thu, 26 Jun 2025 21:56:07 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Mario Limonciello <superm1@...nel.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, 
	Hans de Goede <hansg@...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

On Thu, Jun 26, 2025 at 05:21:35PM -0500, Mario Limonciello wrote:
> On 6/26/2025 2:40 PM, Dmitry Torokhov wrote:
> > On Thu, Jun 26, 2025 at 09:31:12PM +0200, Rafael J. Wysocki wrote:
> > > On Thu, Jun 26, 2025 at 9:28 PM Dmitry Torokhov
> > > <dmitry.torokhov@...il.com> wrote:
> > > > 
> > > > On Thu, Jun 26, 2025 at 09:18:56PM +0200, Rafael J. Wysocki wrote:
> > > > > On Thu, Jun 26, 2025 at 9:16 PM Hans de Goede <hansg@...nel.org> wrote:
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > On 26-Jun-25 21:14, Dmitry Torokhov wrote:
> > > > > > > On Thu, Jun 26, 2025 at 08:57:30PM +0200, Hans de Goede wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On 26-Jun-25 20:48, Dmitry Torokhov wrote:
> > > > > > > > > On Thu, Jun 26, 2025 at 01:20:54PM -0500, Mario Limonciello wrote:
> > > > [...]
> > > > > > > > > > I want to note this driver works quite differently than how ACPI power
> > > > > > > > > > button does.
> > > > > > > > > > 
> > > > > > > > > > You can see in acpi_button_notify() that the "keypress" is only forwarded
> > > > > > > > > > when not suspended [1].  Otherwise it's just wakeup event (which is what my
> > > > > > > > > > patch was modeling).
> > > > > > > > > > 
> > > > > > > > > > https://github.com/torvalds/linux/blob/v6.16-rc3/drivers/acpi/button.c#L461
> > > > > > > > > > [1]
> > > > > > > > > 
> > > > > > > > > If you check acpi_button_resume() you will see that the events are sent
> > > > > > > > > from there. Except that for some reason they chose to use KEY_WAKEUP and
> > > > > > > > > not KEY_POWER, oh well. Unlike acpi button driver gpio_keys is used on
> > > > > > > > > multiple other platforms.
> > > > > > > > 
> > > > > > > > Interesting, but the ACPI button code presumably only does this on resume
> > > > > > > > for a normal press while the system is awake it does use KEY_POWER, right ?
> > > > > > > 
> > > > > > > Yes. It is unclear to me why they chose to mangle the event on wakeup,
> > > > > > > it does not seem to be captured in the email discussions or in the patch
> > > > > > > description.
> > > > > > 
> > > > > > I assume they did this to avoid the immediate re-suspend on wakeup by
> > > > > > power-button issue. GNOME has a workaround for this, but I assume that
> > > > > > some userspace desktop environments are still going to have a problem
> > > > > > with this.
> > > > > 
> > > > > It was done for this reason IIRC, but it should have been documented
> > > > > more thoroughly.
> > > > 
> > > > I assert that it should not have been done and instead dealt with in
> > > > userspace. There are numerous drivers in the kernel emitting
> > > > KEY_POWER. Let userspace decide how to handle this, what keys to ignore,
> > > > what keys to process and when.
> > > 
> > > Please see my last message in this thread (just sent) and see the
> > > changelog of commit 16f70feaabe9 ("ACPI: button: trigger wakeup key
> > > events").
> > > 
> > > This appears to be about cases when no event would be signaled to user
> > > space at all (power button wakeup from ACPI S3).
> > 
> > Ahh, in S3 we do not know if we've been woken up with Sleep or Power
> > button, right? So we can not send the "right" event code and use
> > "neutral" KEY_WAKEUP for both. Is this right?
> > 
> > Thanks.
> > 
> 
> I did some more experiments with this affected system that started this
> thread (which uses s2idle).
> 
> I only applied patch 3 in this series to help the debounce behavior and
> figure out impacts from patch 4 with existing Linux userspace.
> 
> If suspended using systemd in GNOME (click the GUI button) on Ubuntu 24.04
> the GNOME workaround mitigates this problem and no visible impact.
> 
> If I suspend by hand using the kernel interface and then press power button
> to wake:
> 
> # echo mem | sudo tee /sys/power/state:
> 
> * When GNOME is running:
> I get the shutdown popup and it eventually shuts down.
> 
> * When GNOME isn't running (just on a VT):
> System shuts down.

For the latter you may want to raise an issue with systemd, and for the
former I guess it is being too clever and does not activate the
workaround if suspend was not initiated by it? I think Gnome is being
too careful.

Thanks.

-- 
Dmitry