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

[Darren Hart]

On Fri, Jul 10, 2020 at 8:19 AM Grant Likely <grant.likely@xxxxxxxxxxxx> 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@xxxxxxxxxxxxx>

Thanks Grant!

> Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
> Cc: Darren Hart <darren@xxxxxxxxxx>
> Cc: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
>  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
>