On Sat, Aug 27, 2022 at 01:36:29PM +0300, Mazin Al Haddad wrote: > On Wed Aug 24, 2022 at 9:04 AM +03, Greg KH wrote: > > On Wed, Aug 24, 2022 at 01:24:38AM +0300, Mazin Al Haddad wrote: > > > Fixes a bug reported by syzbot, where a warning occurs in usb_submit_urb() > > > due to the wrong endpoint type. There is no check for both the number > > > of endpoints and the type which causes an error as the code tries to > > > send a URB to the wrong endpoint. > > > > > > Fix it by adding a check for the number of endpoints and the > > > direction/type of the endpoints. If the endpoints do not match the > > > expected configuration -ENODEV is returned. > > > > > > Syzkaller report: > > > > > > usb 1-1: BOGUS urb xfer, pipe 3 != type 1 > > > WARNING: CPU: 1 PID: 71 at drivers/usb/core/urb.c:502 usb_submit_urb+0xed2/0x18a0 drivers/usb/core/urb.c:502 > > > Modules linked in: > > > CPU: 1 PID: 71 Comm: kworker/1:2 Not tainted 5.19.0-rc7-syzkaller-00150-g32f02a211b0a #0 > > > Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 06/29/2022 > > > Workqueue: usb_hub_wq hub_event > > > Call Trace: > > > <TASK> > > > ar5523_cmd+0x420/0x790 drivers/net/wireless/ath/ar5523/ar5523.c:275 > > > ar5523_cmd_read drivers/net/wireless/ath/ar5523/ar5523.c:302 [inline] > > > ar5523_host_available drivers/net/wireless/ath/ar5523/ar5523.c:1376 [inline] > > > ar5523_probe+0xc66/0x1da0 drivers/net/wireless/ath/ar5523/ar5523.c:1655 > > > > > > > > > Link: https://syzkaller.appspot.com/bug?extid=1bc2c2afd44f820a669f > > > Reported-and-tested-by: syzbot+1bc2c2afd44f820a669f@xxxxxxxxxxxxxxxxxxxxxxxxx > > > Signed-off-by: Mazin Al Haddad <mazinalhaddad05@xxxxxxxxx> > > > --- > > > drivers/net/wireless/ath/ar5523/ar5523.c | 31 ++++++++++++++++++++++++ > > > 1 file changed, 31 insertions(+) > > > > > > diff --git a/drivers/net/wireless/ath/ar5523/ar5523.c b/drivers/net/wireless/ath/ar5523/ar5523.c > > > index 6f937d2cc126..5451bf9ab9fb 100644 > > > --- a/drivers/net/wireless/ath/ar5523/ar5523.c > > > +++ b/drivers/net/wireless/ath/ar5523/ar5523.c > > > @@ -1581,8 +1581,39 @@ static int ar5523_probe(struct usb_interface *intf, > > > struct usb_device *dev = interface_to_usbdev(intf); > > > struct ieee80211_hw *hw; > > > struct ar5523 *ar; > > > + struct usb_host_interface *host = intf->cur_altsetting; > > > int error = -ENOMEM; > > > > > > + if (host->desc.bNumEndpoints != 4) { > > > + dev_err(&dev->dev, "Wrong number of endpoints\n"); > > > + return -ENODEV; > > > + } > > > + > > > + for (int i = 0; i < host->desc.bNumEndpoints; ++i) { > > > + struct usb_endpoint_descriptor *ep = &host->endpoint[i].desc; > > > + // Check for type of endpoint and direction. > > > + switch (i) { > > > + case 0: > > > + case 1: > > > + if ((ep->bEndpointAddress & USB_DIR_OUT) && > > > + ((ep->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) > > > + == USB_ENDPOINT_XFER_BULK)){ > > > > Did you run your change through checkpatch? > > Yes. > > > We have usb helper functions for all of this, why not use them instead > > of attempting to roll your own? > > Using the helpers is indeed a lot better. I wasn't aware of all of them. > Since find_common_endpoints() won't work here, I used the helpers for > checking direction/type. I don't understand why usb_find_common_endpoints() will not work here. It seems to be very generic and should work just fine. thanks, greg k-h
