linux-kernel - Re: Requesting your attention and expertise regarding a Tablet/Kernel issue

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: <20231108194051.279435-2-nils@nilsfuhler.de>
Date:   Wed,  8 Nov 2023 20:40:52 +0100
From:   Nils Fuhler <nils@...sfuhler.de>
To:     benjamin.tissoires@...hat.com
Cc:     davidrevoy@...tonmail.com, folays@...il.com,
        jason.gerecke@...om.com, jkosina@...e.cz,
        jose.exposito89@...il.com, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org, ostapyshyn@....uni-hannover.de,
        nils@...sfuhler.de
Subject: Re: Requesting your attention and expertise regarding a Tablet/Kernel issue

Benjamin Tissoires <benjamin.tissoires@...hat.com> writes:

> So, to me:
> - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> firmware bug (reporting invert through eraser) and should not be
> tackled at the generic level, thus it should be reverted

Surely that can't be the solution.  [1] is not a specification but a
suggestion that many manufacturers seem to follow.  Apparently, there
are devices that directly report "erasing" for the upper button,
skipping the whole "intent to erase" business.  A questionable decision,
but clearly intended.

The evdev input protocol represents the erasing action as BTN_TOUCH with
BTN_TOOL_RUBBER being active.  Previously, it was assumed that there is
an Invert usage that would toggle BTN_TOOL_RUBBER.  XP-Pen Artist 24
(and possibly other devices) does not have Invert in its report
descriptor, resulting in missing BTN_TOOL_RUBBER.  BTN_TOUCH without an
active tool has no meaning in the input stream.

276e14e6c3 adds a quirk for this and sends the required BTN_TOOL_RUBBER
events *only* for styli not doing it themselves via Invert.  In fact,
276e14e6c3 does not even affect the "two-eraser" XP-Pen Artist Pro 16
Gen 2 and all other devices having Invert in their report descriptors.
The quirk is not set, the behavior is unchanged [2].

As Illia already described, the *real problem* is the missing
compatibility of the whole hidinput_hid_event state machine with
hidinput_setkeycode, invoked from userspace via the EVIOCSKEYCODE ioctl.
In David's case, this is done by hwdb.

This has been the case at least since the refactoring and even affects
styli *completely* adhering to [1]: remapping Invert to something other
than BTN_TOOL_RUBBER borks the device.  If you replay the recording [3]
(with or without 276e14e6c3) and remap [4] 0xd003c (Invert) to something
other than BTN_TOOL_RUBBER, you can observe that:

(1) Remapped Invert does not trigger the event it was remapped to -- this
    is due to hardcoded BTN_TOOL_RUBBER and BTN_TOUCH in hidinput_hid_event

(2) After triggering Invert once, BTN_TOOL_PEN and BTN_TOUCH never appear
    in the event stream again -- remapping Invert masks BTN_TOOL_RUBBER,
    causing it to get permanently stuck in report->tool.

276e14e6c3 does extend this issue onto Eraser remappings for devices
without Invert, resulting in this regression.  However, the solution is
to fix 276e14e6c3 (and the whole function, while we're at it), not to
revert it.

> - both of these tablets are forwarding the useful information, but not
> correctly, which confuses the kernel
> - I should now be able to write regression tests
> - I should be able to provide HID-BPF fixes for those tablets so that
> we can keep them working with or without
> 276e14e6c3993317257e1787e93b7166fbc30905
> reverted (hopefully)
> - problem is I still don't have the mechanics to integrate the HID-BPF
> fixes directly in the kernel tree, so maybe I'll have to write a
> driver for XP-Pen while these internals are set (it shouldn't
> interfere with the HID-BPF out of the tree).

As you can see, there is no need for rewriting anything.  The generic
solution for "invertless" digitizers is already in place and has no
impact on other devices.  IMHO, it would be wiser to fix the regression
by making hidinput_hid_event compatible with EVIOCSKEYCODE, than to miss
the point of what is actually broken and write device-specific drivers.

[1] https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states

[2] David confirms it in his blog post: "I now know it is there for a
    long time, because even with the "old" kernel, my newer XPPen 16 Pro
    (gen2) also reacts this way."
    https://www.davidrevoy.com/article995/how-a-kernel-update-broke-my-stylus-need-help

[3] https://dl.uni-h.de/?t=6b2cabd8f15ac8ff281b52e25920c0a9

[4] https://github.com/iostapyshyn/evmap is a handy EVIOCSKEYCODE wrapper
    for remapping.