[Bug 212995] New: A possible divide by zero in qh_urb_transaction

[Bug 212995] New: A possible divide by zero in qh_urb_transaction

[bugzilla-daemon]

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:

static inline __u16
usb_maxpacket(struct usb_device *udev, int pipe, int is_out)
{
        struct usb_host_endpoint        *ep;
        ...
        if (!ep)
                return 0;
        ...
}

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

Re: [Bug 212995] New: A possible divide by zero in qh_urb_transaction

[Greg KH]

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

[Bug 212995] A possible divide by zero in qh_urb_transaction

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=212995

--- Comment #1 from Greg Kroah-Hartman (greg@kroah.com) ---
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

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 212995] A possible divide by zero in qh_urb_transaction

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=212995

--- Comment #2 from YiyuanGUO (yguoaz@gmail.com) ---
(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.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 212995] A possible divide by zero in qh_urb_transaction

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=212995

--- Comment #3 from Alan Stern (stern@rowland.harvard.edu) ---
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);

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.