[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#964480: linux: ath9k (USB) broken in upstresm linux 5.7.3 after commit 6602f080cb



Juergen <j-r@online.de> writes:

> In 5.7 reverting 6602f080cb is known to fix the problem, but it hasn't yet been
> analyzed by an expert, therefore I'd like to report my findings to perhaps get
> an expert interested:-)
>
> At least one problem is in function ath9k_hif_usb_reg_in_cb(). In the call to
> usb_fill_int_urb() it should be rx_buf instead of nskb as penultimate
> parameter.

Yes, that's an obvious killer bug.  That patch should definitely be
reverted, and replaced with a proper and tested fix.

IMHO, it would be much safer and simpler to validate the expected
descriptors when probing these devices instead of allocating new helper
structs and making changes all over the place.  There is no reason to
add support for the syzbot simulated device.  Returning -ENODEV on probe
is fine, and is much less likely to add new and critical bugs.

And it's not like the fix actually took care of all the hard coded well
known descriptor values in this driver anyway.  We still have stuff like

 usb_sndintpipe(hif_dev->udev, USB_REG_OUT_PIPE)

and

 usb_sndbulkpipe(hif_dev->udev, USB_WLAN_TX_PIPE)

These will not crash the driver, but are still solid indications that
the driver is written for a very specific USB device configuration.  It
will not work with arbitrary descriptors.

I assume the "0" interface is just as fixed as those endpoints, so that
simply validating the values in ath9k_hif_usb_probe() is a perfectly
fine and safe solution.  Without the need to mess up the rest of the
driver.



Bjørn


Reply to: