linux-kernel - Re: [PATCH 0/1] Do not map BTN

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: <CAO-hwJLB8h6fQRF8UjN3rER_6xS2Shi3ffEr92PhkVCijtYRpQ@mail.gmail.com>
Date:   Wed, 24 Nov 2021 10:39:02 +0100
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     José Expósito <jose.exposito89@...il.com>
Cc:     Jiri Kosina <jikos@...nel.org>,
        Peter Hutterer <peter.hutterer@...-t.net>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/1] Do not map BTN_RIGHT/MIDDLE on buttonpads

Hi José,

On Tue, Nov 23, 2021 at 8:12 PM José Expósito <jose.exposito89@...il.com> wrote:
>
> Hi all,
>
> Historically, libinput has relayed on the INPUT_PROP_BUTTONPAD property
> to detect buttonpads.
>
> Since buttonpads are expected to have only one button (BTN_LEFT),
> recently we added a new rule to detect buttonpads: Where a touchpad
> maps the BTN_RIGHT bit, libinput assumes it is NOT a buttonpad.
>
> However, this change leaded to several false possitives, so we ended up
> reverting it. For more context:
> https://gitlab.freedesktop.org/libinput/libinput/-/issues/704
>
> And for a full list of affected hardware, HID reports and bug reports
> please see:
> https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/726
>
> My understanding is that buttonpads should not map BTN_RIGHT and/or
> BTN_MIDDLE and to avoid it I would like to fix the required drivers.

As long as udev intrinsic is happy with it (and it correctly tags the
touchpad as ID_INPUT_something), I'm fine with it.

Also, you might want to point at the specification regarding button
pads: https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report

The way I read it: if the device exports the Button type value
feature, and it is 0 or 1 (click-pad or pressure-pad), there should
not be discrete buttons.

>
> One option to fix it (this patch) is to clear the bits that might have
> been added because of the HID descriptor on every driver.
> However, since this code will be common to all drivers, I would like to
> ask if you consider it worth it to add a function to handle adding
> properties.
>
> A function similar to input_set_capability but for props could be added
> in input.h/c:
>
>     /**
>      * input_set_property - add a property to the device
>      * @dev: device to add the property to
>      * @property: type of the property (INPUT_PROP_POINTER, INPUT_PROP_DIRECT...)
>      *
>      * In addition to setting up corresponding bit in dev->propbit the function
>      * might add or remove related capabilities.
>      */
>     void input_set_property(struct input_dev *dev, unsigned int property)
>     {
>             switch (property) {
>             case INPUT_PROP_POINTER:
>             case INPUT_PROP_DIRECT:
>             case INPUT_PROP_SEMI_MT:
>             case INPUT_PROP_TOPBUTTONPAD:
>             case INPUT_PROP_POINTING_STICK:
>             case INPUT_PROP_ACCELEROMETER:
>                     break;
>
>             case INPUT_PROP_BUTTONPAD:
>                     input_set_capability(dev, EV_KEY, BTN_LEFT);
>                     __clear_bit(BTN_RIGHT, dev->keybit);
>                     __clear_bit(BTN_MIDDLE, dev->keybit);
>                     break;
>
>             default:
>                     pr_err("%s: unknown property %u\n", __func__, property);
>                     dump_stack();
>                     return;
>             }
>
>             __set_bit(property, dev->propbit);
>     }
>     EXPORT_SYMBOL(input_set_property);
>
>
> Which approach do you think is the best?

I think it depends if you plan on fixing just hid-multitouch or the others.
If you have more than one driver, then yes, adding a new symbol in
hid-input.c makes sense. If not, then you are just exposing a new
function we won't know if there are users and we won't be able to
change without care.

Cheers,
Benjamin

>
> Thank you very much in advance,
> Jose
>
>
> José Expósito (1):
>   HID: multitouch: only map BTN_LEFT on buttonpads
>
>  drivers/hid/hid-multitouch.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> --
> 2.25.1
>