linux-kernel - Re: [PATCH] hid-input: Fix devices that return multiple bytes in battery report

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:   Fri, 10 Jul 2020 18:59:42 -0700
From:   Darren Hart <dvhart@...radead.org>
To:     Grant Likely <grant.likely@...retlab.ca>
Cc:     LKML <linux-kernel@...r.kernel.org>, linux-input@...r.kernel.org,
        Grant Likely <grant.likely@....com>,
        Darren Hart <darren@...art.com>,
        Jiri Kosina <jikos@...nel.org>,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] hid-input: Fix devices that return multiple bytes in
 battery report

On Fri, Jul 10, 2020 at 8:19 AM Grant Likely <grant.likely@...retlab.ca> wrote:
>
> Some devices, particularly the 3DConnexion Spacemouse wireless 3D
> controllers, return more than just the battery capacity in the battery
> report. The Spacemouse devices return an additional byte with a device
> specific field. However, hidinput_query_battery_capacity() only
> requests a 2 byte transfer.
>
> When a spacemouse is connected via USB (direct wire, no wireless dongle)
> and it returns a 3 byte report instead of the assumed 2 byte battery
> report the larger transfer confuses and frightens the USB subsystem
> which chooses to ignore the transfer. Then after 2 seconds assume the
> device has stopped responding and reset it. This can be reproduced
> easily by using a wired connection with a wireless spacemouse. The
> Spacemouse will enter a loop of resetting every 2 seconds which can be
> observed in dmesg.
>
> This patch solves the problem by increasing the transfer request to 4
> bytes instead of 2. The fix isn't particularly elegant, but it is simple
> and safe to backport to stable kernels. A further patch will follow to
> more elegantly handle battery reports that contain additional data.
>

Applied and tested on 5.8.0-rc4+ (aa0c9086b40c) with a 3Dconnexion
SpaceMouse Wireless (tested connected via USB). Observed the same
behavior Grant reports before the patch. After the patch, the device stays
connected successfully.

Tested-by: Darren Hart <dvhart@...radead.org>

Thanks Grant!

> Signed-off-by: Grant Likely <grant.likely@...retlab.ca>
> Cc: Darren Hart <darren@...art.com>
> Cc: Jiri Kosina <jikos@...nel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> Cc: stable@...r.kernel.org
> ---
>  drivers/hid/hid-input.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index dea9cc65bf80..e8641ce677e4 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -350,13 +350,13 @@ static int hidinput_query_battery_capacity(struct hid_device *dev)
>         u8 *buf;
>         int ret;
>
> -       buf = kmalloc(2, GFP_KERNEL);
> +       buf = kmalloc(4, GFP_KERNEL);
>         if (!buf)
>                 return -ENOMEM;
>
> -       ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2,
> +       ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 4,
>                                  dev->battery_report_type, HID_REQ_GET_REPORT);
> -       if (ret != 2) {
> +       if (ret < 2) {
>                 kfree(buf);
>                 return -ENODATA;
>         }
> --
> 2.20.1
>