linux-kernel - [BUG REPORT] linux-input: keyboard: gpio_keys: False Button Press Event on Wake

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-next>] [day] [month] [year] [list]

Message-ID: <BL0PR07MB41151EC4B770F6A7396C152EAD570@BL0PR07MB4115.namprd07.prod.outlook.com>
Date:   Wed, 3 Apr 2019 17:50:09 +0000
From:   Ken Sloat <KSloat@...pglobal.com>
To:     "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>
CC:     "josephl@...dia.com" <josephl@...dia.com>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ken Sloat <KSloat@...pglobal.com>
Subject: [BUG REPORT] linux-input: keyboard: gpio_keys: False Button Press
 Event on Wake

Hello Dmitry,

I may have found a potential bug in the "gpio_keys" driver. FYI, I am running the 4.14 LTS kernel on my system, but from my understanding of the issue, it seems that this would still occur in the latest version of the kernel.

The problem:
In the 4.14 LTS kernel, both key press and release events can generate a wake event. In the 5.x kernel, wake events are configurable for press only, release only or "both" (see "wakeup-event-action" binding). The issue can occur in the "both" case or release/deasserted case. Let's imagine that a system is suspended when a gpio key button is pressed, and subsequently resumed when the button is released. If we look at the sequence of actions and events reported by the input system, we can see the potential problem:

Button Pressed
Event Value 1
System Suspend
Button Released
System Wake & Resume
Event Value 0
Event Value 1
Event Value 0

As you can see the input system will report an extra button event/press. This appears to be caused in gpio_keys_gpio_isr by the following statement:

if (bdata->suspended  &&
    (button->type == 0 || button->type == EV_KEY)) {
        /*
         * Simulate wakeup key press in case the key has
         * already released by the time we got interrupt
         * handler to run.
         */
        input_report_key(bdata->input, button->code, 1);
}

This code does not seem to take into account that the wake event may have been caused by a button release action, and just assumes we must have a button press.

This can obviously be problematic in the use case I mentioned, as the system would be put in a constant loop between waking and sleeping. While there are other ways to deal with or react to this issue in the userspace, it seems that the driver should probably take this into account.

Thoughts?

Thanks,
Ken Sloat