From f4abaa9eebde334045ed6ac4e564d050f1df3013 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Tue, 29 Jun 2021 11:25:50 -0700 Subject: HID: input: do not report stylus battery state as "full" The power supply states of discharging, charging, full, etc, represent state of charging, not the capacity level of the battery (for which we have a separate property). Current HID usage tables to not allow for expressing charging state of the batteries found in generic styli, so we should simply assume that the battery is discharging even if current capacity is at 100% when battery strength reporting is done via HID interface. In fact, we were doing just that before commit 581c4484769e. This change helps UIs to not mis-represent fully charged batteries in styli as being charging/topping-off. Fixes: 581c4484769e ("HID: input: map digitizer battery usage") Reported-by: Kenneth Albanowski Signed-off-by: Dmitry Torokhov Signed-off-by: Jiri Kosina --- drivers/hid/hid-input.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index 4286a51f7f16..4b5ebeacd283 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -419,8 +419,6 @@ static int hidinput_get_battery_property(struct power_supply *psy, if (dev->battery_status == HID_BATTERY_UNKNOWN) val->intval = POWER_SUPPLY_STATUS_UNKNOWN; - else if (dev->battery_capacity == 100) - val->intval = POWER_SUPPLY_STATUS_FULL; else val->intval = POWER_SUPPLY_STATUS_DISCHARGING; break; -- cgit v1.2.3 From f7744fa16b96da57187dc8e5634152d3b63d72de Mon Sep 17 00:00:00 2001 From: Anirudh Rayabharam Date: Thu, 24 Jun 2021 00:10:29 +0530 Subject: HID: usbhid: free raw_report buffers in usbhid_stop Free the unsent raw_report buffers when the device is removed. Fixes a memory leak reported by syzbot at: https://syzkaller.appspot.com/bug?id=7b4fa7cb1a7c2d3342a2a8a6c53371c8c418ab47 Reported-by: syzbot+47b26cd837ececfc666d@syzkaller.appspotmail.com Tested-by: syzbot+47b26cd837ececfc666d@syzkaller.appspotmail.com Signed-off-by: Anirudh Rayabharam Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'drivers/hid') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 06130dc431a0..0bf123bf2ef8 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -505,7 +505,7 @@ static void hid_ctrl(struct urb *urb) if (unplug) { usbhid->ctrltail = usbhid->ctrlhead; - } else { + } else if (usbhid->ctrlhead != usbhid->ctrltail) { usbhid->ctrltail = (usbhid->ctrltail + 1) & (HID_CONTROL_FIFO_SIZE - 1); if (usbhid->ctrlhead != usbhid->ctrltail && @@ -1223,9 +1223,20 @@ static void usbhid_stop(struct hid_device *hid) mutex_lock(&usbhid->mutex); clear_bit(HID_STARTED, &usbhid->iofl); + spin_lock_irq(&usbhid->lock); /* Sync with error and led handlers */ set_bit(HID_DISCONNECTED, &usbhid->iofl); + while (usbhid->ctrltail != usbhid->ctrlhead) { + if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_OUT) { + kfree(usbhid->ctrl[usbhid->ctrltail].raw_report); + usbhid->ctrl[usbhid->ctrltail].raw_report = NULL; + } + + usbhid->ctrltail = (usbhid->ctrltail + 1) & + (HID_CONTROL_FIFO_SIZE - 1); + } spin_unlock_irq(&usbhid->lock); + usb_kill_urb(usbhid->urbin); usb_kill_urb(usbhid->urbout); usb_kill_urb(usbhid->urbctrl); -- cgit v1.2.3 From 5049307d37a760e304ad191c5dc7c6851266d2f8 Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Wed, 1 Sep 2021 12:35:49 -0400 Subject: HID: usbhid: Fix flood of "control queue full" messages [patch description by Alan Stern] Commit 7652dd2c5cb7 ("USB: core: Check buffer length matches wLength for control transfers") causes control URB submissions to fail if the transfer_buffer_length value disagrees with the setup packet's wLength valuel. Unfortunately, it turns out that the usbhid can trigger this failure mode when it submits a control request for an input report: It pads the transfer buffer size to a multiple of the maxpacket value but does not increase wLength correspondingly. These failures have caused problems for people using an APS UPC, in the form of a flood of log messages resembling: hid-generic 0003:051D:0002.0002: control queue full This patch fixes the problem by setting the wLength value equal to the padded transfer_buffer_length value in hid_submit_ctrl(). As a nice bonus, the code which stores the transfer_buffer_length value is now shared between the two branches of an "if" statement, so it can be de-duplicated. Signed-off-by: Michal Kubecek Signed-off-by: Alan Stern Fixes: 7652dd2c5cb7 ("USB: core: Check buffer length matches wLength for control transfers") Tested-by: Oleksandr Natalenko Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Cc: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 0bf123bf2ef8..6b8690878435 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -377,27 +377,26 @@ static int hid_submit_ctrl(struct hid_device *hid) len = hid_report_len(report); if (dir == USB_DIR_OUT) { usbhid->urbctrl->pipe = usb_sndctrlpipe(hid_to_usb_dev(hid), 0); - usbhid->urbctrl->transfer_buffer_length = len; if (raw_report) { memcpy(usbhid->ctrlbuf, raw_report, len); kfree(raw_report); usbhid->ctrl[usbhid->ctrltail].raw_report = NULL; } } else { - int maxpacket, padlen; + int maxpacket; usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0); maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); if (maxpacket > 0) { - padlen = DIV_ROUND_UP(len, maxpacket); - padlen *= maxpacket; - if (padlen > usbhid->bufsize) - padlen = usbhid->bufsize; + len = DIV_ROUND_UP(len, maxpacket); + len *= maxpacket; + if (len > usbhid->bufsize) + len = usbhid->bufsize; } else - padlen = 0; - usbhid->urbctrl->transfer_buffer_length = padlen; + len = 0; } + usbhid->urbctrl->transfer_buffer_length = len; usbhid->urbctrl->dev = hid_to_usb_dev(hid); usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir; -- cgit v1.2.3 From 0a824efdb724e07574bafcd2c2486b2a3de35ff6 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 1 Sep 2021 12:36:00 -0400 Subject: HID: usbhid: Fix warning caused by 0-length input reports Syzbot found a warning caused by hid_submit_ctrl() submitting a control request to transfer a 0-length input report: usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1 (The warning message is a little difficult to understand. It means that the control request claims to be for an IN transfer but this contradicts the USB spec, which requires 0-length control transfers always to be in the OUT direction.) Now, a zero-length report isn't good for anything and there's no reason for a device to have one, but the fuzzer likes to pick out these weird edge cases. In the future, perhaps we will decide to reject 0-length reports at probe time. For now, the simplest approach for avoiding these warnings is to pretend that the report actually has length 1. Signed-off-by: Alan Stern Reported-and-tested-by: syzbot+9b57a46bf1801ce2a2ca@syzkaller.appspotmail.com Tested-by: Oleksandr Natalenko Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Cc: stable@vger.kernel.org Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/hid') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 6b8690878435..c56cb03c1551 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -389,6 +389,7 @@ static int hid_submit_ctrl(struct hid_device *hid) maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); if (maxpacket > 0) { + len += (len == 0); /* Don't allow 0-length reports */ len = DIV_ROUND_UP(len, maxpacket); len *= maxpacket; if (len > usbhid->bufsize) -- cgit v1.2.3 From d2f311ec91984adb219ac6985d4dd72c37ae734d Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Wed, 1 Sep 2021 12:36:06 -0400 Subject: HID: usbhid: Simplify code in hid_submit_ctrl() This patch makes a small simplification to the code in hid_submit_ctrl(). The test for maxpacket being > 0 is unnecessary, because endpoint 0 always has a maxpacket value which is >= 8. Furthermore, endpoint 0's maxpacket value is always a power of 2, so instead of open-coding the round-to-next-multiple computation we can call the optimized round_up() routine. Signed-off-by: Alan Stern Tested-by: Benjamin Tissoires Acked-by: Benjamin Tissoires Signed-off-by: Jiri Kosina --- drivers/hid/usbhid/hid-core.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index c56cb03c1551..2dcaf31eb9cd 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -388,14 +388,10 @@ static int hid_submit_ctrl(struct hid_device *hid) usbhid->urbctrl->pipe = usb_rcvctrlpipe(hid_to_usb_dev(hid), 0); maxpacket = usb_maxpacket(hid_to_usb_dev(hid), usbhid->urbctrl->pipe, 0); - if (maxpacket > 0) { - len += (len == 0); /* Don't allow 0-length reports */ - len = DIV_ROUND_UP(len, maxpacket); - len *= maxpacket; - if (len > usbhid->bufsize) - len = usbhid->bufsize; - } else - len = 0; + len += (len == 0); /* Don't allow 0-length reports */ + len = round_up(len, maxpacket); + if (len > usbhid->bufsize) + len = usbhid->bufsize; } usbhid->urbctrl->transfer_buffer_length = len; usbhid->urbctrl->dev = hid_to_usb_dev(hid); -- cgit v1.2.3