Skip to contentSkip to conversation

New: A possible divide by zero in qh_urb_transaction

Bug 212995 [email protected] 2 participants 5 messages May 8, 2021 USB

lore
Share & tools
lore

[email protected]

author
#0 lore raw

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: [email protected]
          Reporter: [email protected]
        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;
        ...
}
Signature · 4 lines
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.
#1 lore raw
Quoted 22 lines · On Sat, May 08, 2021 at 11:14:25AM +0000, [email protected]…

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: [email protected]
          Reporter: [email protected]
        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):

Quoted 7 lines

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

author
#2 lore raw

[Bug 212995] A possible divide by zero in qh_urb_transaction

In reply to [email protected]

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

--- Comment #1 from Greg Kroah-Hartman ([email protected]) ---

Quoted 24 lines · On Sat, May 08, 2021 at 11:14:25AM +0000, [email protected]…

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: [email protected]
          Reporter: [email protected]
        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):

Quoted 7 lines

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

Signature · 4 lines
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.
author
#3 lore raw

[Bug 212995] A possible divide by zero in qh_urb_transaction

In reply to [email protected]

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

--- Comment #2 from YiyuanGUO ([email protected]) --- (In reply to Greg Kroah-Hartman from comment #1)

Quoted 36 lines
Quoted 18 lines · On Sat, May 08, 2021 at 11:14:25AM +0000, [email protected]…

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: [email protected]
          Reporter: [email protected]
        Regression: No
Quoted 5 lines

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):

Quoted 7 lines

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

Quoted 1 line

zero problem in the remainder operation:

Quoted 7 lines

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.

Signature · 4 lines
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.
author
#4 lore raw

[Bug 212995] A possible divide by zero in qh_urb_transaction

In reply to [email protected]

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

--- Comment #3 from Alan Stern ([email protected]) ---
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);

Signature · 4 lines
You may reply to this email to add a comment.

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