linux-kernel - Re: [PATCH v3 0/3] new driver for Valve Steam Controller

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: <CAM4jgCpX1OOBqCEvqtyx=ftvnuk3zg_jMsFkxFZXxEfZD-ak5Q@mail.gmail.com>
Date:   Mon, 26 Feb 2018 12:24:21 +0100
From:   Clément VUCHENER <clement.vuchener@...il.com>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:     Rodrigo Rivas Costa <rodrigorivascosta@...il.com>,
        Jiri Kosina <jikos@...nel.org>,
        "Pierre-Loup A. Griffais" <pgriffais@...vesoftware.com>,
        Cameron Gutman <aicommander@...il.com>,
        lkml <linux-kernel@...r.kernel.org>,
        linux-input <linux-input@...r.kernel.org>
Subject: Re: [PATCH v3 0/3] new driver for Valve Steam Controller

2018-02-26 10:50 GMT+01:00 Benjamin Tissoires <benjamin.tissoires@...hat.com>:
> Hi Rodrigo,
>
> On Sun, Feb 25, 2018 at 7:52 PM, Rodrigo Rivas Costa
> <rodrigorivascosta@...il.com> wrote:
>> This patchset implements a driver for Valve Steam Controller, based on a
>> reverse analysis by myself.
>
> To me, the code looks OK now. I haven't got the time to do a better
> review, so giving my:
> Acked-by: Bnejamin Tissoires <benjamin.tissoires@...hat.com>
>
> However, before we include it, I'd like to have the ACK from
> Pierre-Loup and Clément. They should be more qualified than me to say
> if this will interfere with the official Steam client (I think we are
> good now, but that's just my opinion).

I am not qualified to speak about the steam client, it has been a long
time since I have a look at how it uses the controller and I think it
changed a lot.

I checked the code and I think you are not handling wireless
connection correctly. If a wireless controller is already connected
when the driver is loaded, you will not receive a connection event and
forget to register the controller. You can send a 0xb4 request with
empty parameters (0xb4, 0x00, ... ) to force the receiver to send its
connection status (as the type 3 event you are already parsing). Since
user-space program may also send this request, you should make sure
the driver works if it receives a (dis)connected event when it is
already (dis)connected.

>
> Cheers,
> Benjamin
>
>>
>> This is reroll v3, codenamed "lazy lizard". Changes from v2:
>>  * Use RCU to do the dynamic connec/disconnect of wireless devices. Please,
>>    to anybody that knows their way around RCU, review.
>>  * Remove entries in hid-quirks.c as they are no longer needed. This allows
>>    this module to be blacklisted without side effects.
>>  * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
>>    existing use cases (lizard mode). A user-space tool to do that is
>>    linked.
>>  * Fully separated axes for joystick and left-pad. As it happens, there are
>>    people with too many fingers.
>>  * Add fuzz values for left/right pad axes, they are a little wiggly.
>>
>> Notable changes from patchset v1:
>>  * Remove references to USB. Now the interesting interfaces are selected by
>>    looking for the ones with feature reports.
>>  * Feature reports buffers are allocated with hid_alloc_report_buf().
>>  * Feature report length is checked, to avoid overflows in case of
>>    corrupt/malicius USB devices.
>>  * Resolution added to the ABS axes.
>>  * A lot of minor cleanups.
>>
>> Rodrigo Rivas Costa (3):
>>   HID: add driver for Valve Steam Controller
>>   HID: steam: add serial number information.
>>   HID: steam: add battery device.
>>
>>  drivers/hid/Kconfig     |   8 +
>>  drivers/hid/Makefile    |   1 +
>>  drivers/hid/hid-ids.h   |   4 +
>>  drivers/hid/hid-steam.c | 777 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 790 insertions(+)
>>  create mode 100644 drivers/hid/hid-steam.c
>>
>> --
>> 2.16.2
>>