linux-kernel - Re: [PATCH 3/4] HID: multitouch: drop reports containing invalid values

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]

Date:   Tue, 3 Jul 2018 10:13:54 +0200
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Joey Pabalinas <joeypabalinas@...il.com>
Cc:     "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Jiri Kosina <jikos@...nel.org>,
        Daniel Martin <consume.noise@...il.com>
Subject: Re: [PATCH 3/4] HID: multitouch: drop reports containing invalid values

Hi Joey,

On Sun, Jul 1, 2018 at 2:19 AM, Joey Pabalinas <joeypabalinas@...il.com> wrote:
> Avoid processing reports containing invalid values to reduce
> multitouch input stutter.
>
> Signed-off-by: Joey Pabalinas <joeypabalinas@...il.com>
>
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index c0654db0b736543ca0..08b50e5908cecdda66 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -694,10 +694,19 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>  {
>         if ((td->mtclass.quirks & MT_QUIRK_CONTACT_CNT_ACCURATE) &&
>             td->num_received >= td->num_expected)
>                 return;
>
> +       /* drop invalid values after counting them */
> +       if (td->curdata.x == 0xffff &&
> +                       td->curdata.y == 0xffff &&
> +                       td->curdata.w == 0xffff &&
> +                       td->curdata.h == 0xffff) {

You can't really use plain values like that. There is a tiny chance
these values are valid on an other device.
IIRC, MS spec says that we should ignore out of band values if they
are tagged as such. Such input are tagged with NULL values
(http://www.usb.org/developers/hidpage/HID1_11.pdf page 31) and MS
spec mentioned this.

All in all, if you have this bit set, you need to compare the value
with the logical_max/min for each field.

I never encountered a device that required this, so you are probably
the lucky one :)

Cheers,
Benjamin

> +               td->num_received++;
> +               return;
> +       }
> +
>         if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
>                 int active;
>                 int slotnum = mt_compute_slot(td, input);
>                 struct mt_slot *s = &td->curdata;
>                 struct input_mt *mt = input->mt;
> --
> 2.18.0
>