Re: [REGRESSION][BISECTED] flood of "hid-generic ... control queue full" since v5.14-rc1

From: Michal Kubecek
Date: Mon Aug 16 2021 - 15:13:43 EST


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;

Attachment: signature.asc
Description: PGP signature