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.
Signed-off-by: Grant Likely <[email protected]>
Cc: Darren Hart <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Benjamin Tissoires <[email protected]>
Cc: [email protected]
---
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
On Fri, Jul 10, 2020 at 8:19 AM Grant Likely <[email protected]> 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 <[email protected]>
Thanks Grant!
> Signed-off-by: Grant Likely <[email protected]>
> Cc: Darren Hart <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: Benjamin Tissoires <[email protected]>
> Cc: [email protected]
> ---
> 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
>
On Fri, 10 Jul 2020, Darren Hart 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 <[email protected]>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
Hi
[This is an automated email]
This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all
The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230, v4.4.230.
v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Build OK!
v4.14.188: Build OK!
v4.9.230: Failed to apply! Possible dependencies:
581c4484769e6 ("HID: input: map digitizer battery usage")
v4.4.230: Failed to apply! Possible dependencies:
581c4484769e6 ("HID: input: map digitizer battery usage")
5d9374cf5f66e ("HID: input: ignore the battery in OKLICK Laser BTmouse")
NOTE: The patch will not be queued to stable trees until it is upstream.
How should we proceed with this patch?
--
Thanks
Sasha