linux-kernel - Re: [v3 4/5] HID: logitech-hidpp: Fix "Sw. Id." for HID++ 2.0 commands

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-hwJJWDFS7zCbGAvbDCx0e+xNCtr_A4gBx0T5M8WeDVhnPgA@mail.gmail.com>
Date:   Tue, 30 Aug 2022 15:19:15 +0200
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Bastien Nocera <hadess@...ess.net>
Cc:     "open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Jiri Kosina <jikos@...nel.org>,
        "Peter F . Patel-Schneider" <pfpschneider@...il.com>,
        Filipe Laíns <lains@...eup.net>,
        Nestor Lopez Casado <nlopezcasad@...itech.com>
Subject: Re: [v3 4/5] HID: logitech-hidpp: Fix "Sw. Id." for HID++ 2.0 commands

On Tue, Aug 30, 2022 at 1:39 PM Bastien Nocera <hadess@...ess.net> wrote:
>
> Always set a non-zero "Sw. Id." in the lower nibble of the Function/ASE
> and Software Identifier byte in HID++ 2.0 commands.
>
> As per the "Protocol HID++2.0 essential features" section in
> https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf
> "
> Software identifier (4 bits, unsigned)
>
> A number uniquely defining the software that sends a request. The
> firmware must copy the software identifier in the response but does
> not use it in any other ways.
>
> 0 Do not use (allows to distinguish a notification from a response).
> "
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215699
> Signed-off-by: Bastien Nocera <hadess@...ess.net>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 98ebedb73d98..9c8088d8879e 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -41,6 +41,9 @@ module_param(disable_tap_to_click, bool, 0644);
>  MODULE_PARM_DESC(disable_tap_to_click,
>         "Disable Tap-To-Click mode reporting for touchpads (only on the K400 currently).");
>
> +/* Define a non-zero software ID to identify our own requests */
> +#define LINUX_KERNEL_SW_ID                     0x06

For consistency, and as Peter already asked, please use 0x01 instead of 0x06.

The simple reason is that it was well known that the kernel used 0x01
from day one, and so we might have userspace application that uses
0x06, and in this case you are walking on their toes.

Cheers,
Benjamin

> +
>  #define REPORT_ID_HIDPP_SHORT                  0x10
>  #define REPORT_ID_HIDPP_LONG                   0x11
>  #define REPORT_ID_HIDPP_VERY_LONG              0x12
> @@ -343,7 +346,7 @@ static int hidpp_send_fap_command_sync(struct hidpp_device *hidpp,
>         else
>                 message->report_id = REPORT_ID_HIDPP_LONG;
>         message->fap.feature_index = feat_index;
> -       message->fap.funcindex_clientid = funcindex_clientid;
> +       message->fap.funcindex_clientid = funcindex_clientid | LINUX_KERNEL_SW_ID;
>         memcpy(&message->fap.params, params, param_count);
>
>         ret = hidpp_send_message_sync(hidpp, message, response);
> --
> 2.37.2
>