Re: WARNING in hif_usb_alloc_rx_urbs/usb_submit_urb

From: Fedor Pchelkin
Date: Fri Aug 26 2022 - 09:15:09 EST


Sat, 10 Oct 2020 at 04:08:19 UTC+3, Alan Stern wrote:
> Index: usb-devel/drivers/net/wireless/ath/ath9k/hif_usb.c
> ===================================================================
> --- usb-devel.orig/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ usb-devel/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -1307,6 +1307,20 @@ static int ath9k_hif_usb_probe(struct us
> struct usb_device *udev = interface_to_usbdev(interface);
> struct hif_device_usb *hif_dev;
> int ret = 0;
> + struct usb_host_interface *alt;
> + struct usb_endpoint_descriptor *epd;
> +
> + /* Verify the expected endpoints are present */
> + alt = interface->cur_altsetting;
> + if (!usb_find_int_in_endpoint(alt, &epd) ||
> + usb_endpoint_num(epd) != USB_REG_IN_PIPE ||
> + !usb_find_int_out_endpoint(alt, &epd) ||
> + usb_endpoint_num(epd) != USB_REG_OUT_PIPE ||
> + !usb_find_bulk_in_endpoint(alt, &epd) ||
> + usb_endpoint_num(epd) != USB_WLAN_RX_PIPE ||
> + !usb_find_bulk_out_endpoint(alt, &epd) ||
> + usb_endpoint_num(epd) != USB_WLAN_TX_PIPE)
> + return -ENODEV;
>
> if (id->driver_info == STORAGE_DEVICE)
> return send_eject_command(interface);

We've tested the suggested patch and found a null-ptr-deref. The thing
is that usb_find_{...}_endpoint() returns zero in normal case, and
non-zero value (-ENXIO) when failed (in current patch version it is
supposed to be just opposite and sometimes a NULL epd is dereferenced).
To fix it the negation signs before usb_find_{...}_endpoint() should be
removed.

And we also think usb_find_common_endpoints(...) should be used directly
as all the scanned usb_endpoint_descriptors will be passed to it and
returned in just one call.

If you wish, I may prepare the patch myself.

Fedor