On 2021-08-16 21:12, Michal Kubecek wrote:
On Mon, Aug 16, 2021 at 10:38:56AM -0400, Alan Stern wrote:
On Mon, Aug 16, 2021 at 04:13:47PM +0200, Michal Kubecek wrote:
Looking at the code, the primary problem seems to be that the "else"
branch in hid_submit_ctrl() recalculates transfer_buffer_length to
a rounded up value but assigns the original length to wLength.
Looks like you found the bug. Fixing it might be as simple as setting
len = padlen in that "else" branch. You could then combine the
transfer_buffer_length assignment with the one in the "if" branch and
hoist them out after the entire "if" statement.
With the patch below, there are no errors and the UPS communication
works correctly and so do other HID devices. But I would prefere someone
familiar with HID code to confirm that this is what we want and what
would be the right way to handle usb_submit_urb() errors.
Michal
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 06130dc431a0..ef240ef63a66 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;
Got the same flood of messages when trying 5.14.0, found this thread, applied
the patch & rebuilt, problem is gone & UPS still working fine. Yay.
cheers
Holger
