linux-kernel - Re: WARNING in hif_usb_alloc_rx_urbs/usb_submit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5dce2e1c-fa65-2fb3-08ad-65122f7e495d@ispras.ru>
Date:   Fri, 26 Aug 2022 16:14:48 +0300
From:   Fedor Pchelkin <pchelkin@...ras.ru>
To:     Alan Stern <stern@...land.harvard.edu>
Cc:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        ath9k-devel@....qualcomm.com, ldv-project@...uxtesting.org,
        eli.billauer@...il.com,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        andreyknvl@...gle.com, gustavoars@...nel.org,
        ingrassia@...genesys.com, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-wireless@...r.kernel.org,
        oneukum@...e.com, tiwai@...e.de, syzkaller-bugs@...glegroups.com
Subject: Re: WARNING in hif_usb_alloc_rx_urbs/usb_submit_urb

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