Hello. On čtvrtek 19. srpna 2021 21:53:00 CEST Alan Stern wrote: > On Thu, Aug 19, 2021 at 10:35:11AM -0700, syzbot wrote: > > Hello, > > > > syzbot has tested the proposed patch but the reproducer is still > > triggering an issue: WARNING in hid_submit_ctrl/usb_submit_urb > > > > cm6533_jd 0003:0D8C:0022.0001: submit_ctrl: maxpacket 64 len 0 padlen 0 > > ------------[ cut here ]------------ > > usb 1-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType a1 > > Ah. The padding code doesn't add anything if the length is > already a multiple of the maxpacket value, and of course 0 is such > a multiple. > > The following simplified variant of Michal's patch should fix the > problem. > > Alan Stern > > #syz test: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > 794c7931a242 > > Index: usb-devel/drivers/hid/usbhid/hid-core.c > =================================================================== > --- usb-devel.orig/drivers/hid/usbhid/hid-core.c > +++ usb-devel/drivers/hid/usbhid/hid-core.c > @@ -377,27 +377,23 @@ static int hid_submit_ctrl(struct hid_de > 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; > - } else > - padlen = 0; > - usbhid->urbctrl->transfer_buffer_length = padlen; > + 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); > > usbhid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir; I've tried both Michal's patch as well as this one, and both work for me, hence feel free to add this: Tested-by: Oleksandr Natalenko <oleksandr@xxxxxxxxxxxxxx> once the fix is submitted. Thanks! -- Oleksandr Natalenko (post-factum)
