In the file drivers/usb/host/ehci-q.c, the function qh_urb_transaction has the following code (link to the code location: https://github.com/torvalds/linux/blob/dd860052c99b1e088352bdd4fb7aef46f8d2ef47/drivers/usb/host/ehci-q.c#L715): maxpacket = usb_maxpacket(urb->dev, urb->pipe, !is_input); ... if (usb_pipeout(urb->pipe) && (urb->transfer_flags & URB_ZERO_PACKET) && !(urb->transfer_buffer_length % maxpacket)) However, the function usb_maxpacket may retun 0, leading to a possible divide zero problem in the remainder operation: static inline __u16 usb_maxpacket(struct usb_device *udev, int pipe, int is_out) { struct usb_host_endpoint *ep; ... if (!ep) return 0; ... }
On Sat, May 08, 2021 at 11:14:25AM +0000, bugzilla-daemon@bugzilla.kernel.org wrote: > https://bugzilla.kernel.org/show_bug.cgi?id=212995 > > Bug ID: 212995 > Summary: A possible divide by zero in qh_urb_transaction > Product: Drivers > Version: 2.5 > Kernel Version: 5.12.2 > Hardware: All > OS: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: USB > Assignee: drivers_usb@kernel-bugs.kernel.org > Reporter: yguoaz@gmail.com > Regression: No > > In the file drivers/usb/host/ehci-q.c, the function qh_urb_transaction has > the > following code (link to the code location: > > https://github.com/torvalds/linux/blob/dd860052c99b1e088352bdd4fb7aef46f8d2ef47/drivers/usb/host/ehci-q.c#L715): > > maxpacket = usb_maxpacket(urb->dev, urb->pipe, !is_input); > ... > if (usb_pipeout(urb->pipe) && (urb->transfer_flags & URB_ZERO_PACKET) > && !(urb->transfer_buffer_length % maxpacket)) > > However, the function usb_maxpacket may retun 0, leading to a possible divide > zero problem in the remainder operation: Please feel free to submit a patch to the developers, but first see how it could be possible for usb_maxpacket to ever return 0 for this. Are you sure it's not already sanitized by the time it gets here? thanks, greg k-h
(In reply to Greg Kroah-Hartman from comment #1) > On Sat, May 08, 2021 at 11:14:25AM +0000, > bugzilla-daemon@bugzilla.kernel.org wrote: > > https://bugzilla.kernel.org/show_bug.cgi?id=212995 > > > > Bug ID: 212995 > > Summary: A possible divide by zero in qh_urb_transaction > > Product: Drivers > > Version: 2.5 > > Kernel Version: 5.12.2 > > Hardware: All > > OS: Linux > > Tree: Mainline > > Status: NEW > > Severity: normal > > Priority: P1 > > Component: USB > > Assignee: drivers_usb@kernel-bugs.kernel.org > > Reporter: yguoaz@gmail.com > > Regression: No > > > > In the file drivers/usb/host/ehci-q.c, the function qh_urb_transaction has > > the > > following code (link to the code location: > > > > > https://github.com/torvalds/linux/blob/dd860052c99b1e088352bdd4fb7aef46f8d2ef47/drivers/usb/host/ehci-q.c#L715): > > > > maxpacket = usb_maxpacket(urb->dev, urb->pipe, !is_input); > > ... > > if (usb_pipeout(urb->pipe) && (urb->transfer_flags & URB_ZERO_PACKET) > > && !(urb->transfer_buffer_length % maxpacket)) > > > > However, the function usb_maxpacket may retun 0, leading to a possible > divide > > zero problem in the remainder operation: > > Please feel free to submit a patch to the developers, but first see how > it could be possible for usb_maxpacket to ever return 0 for this. Are > you sure it's not already sanitized by the time it gets here? > > thanks, > > greg k-h To avoid any problem, the code should ensure that urb->dev->ep_out[epnum] and urb->dev->ep_in[epnum] are not NULL, a case that is explicitly checked inside usb_maxpacket. It seems to me that there is not direct protection against the field urb->dev.
The code _does_ make this check. See usb_submit_urb(); near the start it does: ep = usb_pipe_endpoint(dev, urb->pipe); if (!ep) return -ENOENT; Now if you want to make this a little clearer, you could submit a patch that changes the code in qh_urb_transaction() to do this: maxpacket = usb_endpoint_maxp(&urb->ep->desc);