On Wed, Aug 24, 2022 at 12:27:57AM +0600, Khalid Masum wrote:
> usb_maxpacket() returns 0 if it fails to fetch the endpoint. This
> value is later used for calculating modulo. Which can cause modulo
> by zero in qtd_fill and qh_urb_transaction.
>
> Prevent this breakage by returning if maxpacket is found to be 0.
>
> Fixes coverity warning: 744857 ("Division or modulo by zero")
> Signed-off-by: Khalid Masum <khalid.masum.92@xxxxxxxxx>
I'm sure we've seen at least one patch doing this submitted in the past.
It was wrong then and it's wrong now. Coverity doesn't have a full
understanding of how the kernel's USB subsystem works and sometimes it
makes mistakes.
In short, qh_urb_transaction() can be called only by pathways that pass
through usb_submit_urb(), which already includes this check:
ep = usb_pipe_endpoint(dev, urb->pipe);
if (!ep)
return -ENOENT;
There's no need to check it again in the ehci-hcd driver.
On Wed, Aug 24, 2022 at 12:27:58AM +0600, Khalid Masum wrote:
> usb_maxpacket() returns 0 if it fails to fetch the endpoint. This
> value is later used for calculating modulo. Which can cause modulo
> by zero in qtd_fill.
>
> Prevent this breakage by returning if maxpacket is found to be 0.
>
> Fixes coverity warning: 1487371 ("Division or modulo by zero")
> Fixes: 9841f37a1cca ("usb: ehci: Add support for SINGLE_STEP_SET_FEATURE test of EHSET")
> Signed-off-by: Khalid Masum <khalid.masum.92@xxxxxxxxx>
This also is unnecessary. Calls to
ehci_submit_single_step_set_feature() have to pass through
request_single_step_set_feature_urb(), which already includes this
check:
if (!ep) {
usb_free_urb(urb);
return NULL;
}
Neither of these patches is needed.
Alan Stern
