linux-kernel - Re: [PATCH] HID: fix A4Tech horizontal scrolling

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: <20190507050150.GA9838@jelly>
Date:   Tue, 7 May 2019 15:01:51 +1000
From:   Peter Hutterer <peter.hutterer@...-t.net>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Igor Kushnir <igorkuo@...il.com>,
        Błażej Szczygieł <spaz16@...pl>,
        Jiri Kosina <jikos@...nel.org>,
        "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] HID: fix A4Tech horizontal scrolling

On Fri, May 03, 2019 at 01:59:23PM +0200, Benjamin Tissoires wrote:
> Hi,
> 
> On Fri, May 3, 2019 at 11:43 AM Igor Kushnir <igorkuo@...il.com> wrote:
> >
> > Hi Benjamin,
> >
> > On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
> > > Hi,
> > >
> > > On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@...pl> wrote:
> > >>
> > >> Since recent high resolution scrolling changes the A4Tech driver must
> > >> check for the "REL_WHEEL_HI_RES" usage code.
> > >>
> > >> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
> > >> Resolution Multiplier for high-resolution scrolling)
> > >>
> > >> Signed-off-by: Błażej Szczygieł <spaz16@...pl>
> > >
> > > Thanks for the patch. I do not doubt this fixes the issues, but I
> > > still wonder if we should not export REL_HWHEEL_HI_RES instead of
> > > REL_HWHEEL events.
> >
> >
> > If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
> > hid-a4tech.c, then it makes sense to me, though I do not know the code
> > well enough to be certain.
> 
> Yep, that's what I meant. I am worried that userspace doesn't know
> well how to deal with a device that mixes the new and old REL_WHEEL
> events.

sorry, I'm not sure what you mean here. The new events are always mixed with
the old ones anyway, and both should be treated as separate event streams.
The kernel interface to userspace is fairly easy to deal with, it's the rest
that's a bit of mess.

[..]

> >
> 
> OK, thanks both of you for your logs, this is helpful.
> So just in case I need to come back later, the horizontal wheel is
> "just" the normal wheel plus a modifier in the report.
> 
> Anyway, ideally, can we have a v2 of the patch with the 2 changes
> requested above in the commit message and the introduction of
> REL_HWHEEL_HI_RES events in addition to REL_HWHEEL?
> REL_HWHEEL_HI_RES should report `120*value` and we should also keep
> the reporting of REL_WHEEL as it is currently.
> 
> Peter, I grepped in the hid code, and it seems hid-cypress.c is having
> the exact same issue. Sigh.

yeah, I found that too when grepping through it. seems to be the only other
one though and we can use Błażej's patch as boilerplate once it's done.

Cheers,
   Peter