On Fri, May 21, 2021 at 05:07:34PM +0800, Hayes Wang wrote: > Verify some fields of the USB descriptor to make sure the driver > could be used by the device. > > BugLink: https://syzkaller.appspot.com/bug?id=912c9c373656996801b4de61f1e3cb326fe940aa > Reported-by: syzbot+95afd23673f5dd295c57@xxxxxxxxxxxxxxxxxxxxxxxxx > Fixes: c2198943e33b ("r8152: search the configuration of vendor mode") > Signed-off-by: Hayes Wang <hayeswang@xxxxxxxxxxx> > --- > drivers/net/usb/r8152.c | 71 +++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 69 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c > index 136ea06540ff..f348350f5da1 100644 > --- a/drivers/net/usb/r8152.c > +++ b/drivers/net/usb/r8152.c > @@ -8107,6 +8107,69 @@ static void r8156b_init(struct r8152 *tp) > tp->coalesce = 15000; /* 15 us */ > } > > +static bool rtl_check_vendor_ok(struct usb_interface *intf) > +{ > + struct usb_host_interface *alt = intf->cur_altsetting; > + struct usb_host_endpoint *in = NULL, *out = NULL, *intr = NULL; > + unsigned int ep; > + > + if (alt->desc.bNumEndpoints < 3) { > + dev_err(&intf->dev, "Unexpected bNumEndpoints %d\n", alt->desc.bNumEndpoints); > + return false; > + } > + > + for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) { > + struct usb_host_endpoint *e; > + > + e = alt->endpoint + ep; > + > + /* ignore endpoints which cannot transfer data */ > + if (!usb_endpoint_maxp(&e->desc)) > + continue; > + > + switch (e->desc.bmAttributes) { > + case USB_ENDPOINT_XFER_INT: > + if (!usb_endpoint_dir_in(&e->desc)) > + continue; > + if (!intr) > + intr = e; > + break; > + case USB_ENDPOINT_XFER_BULK: > + if (usb_endpoint_dir_in(&e->desc)) { > + if (!in) > + in = e; > + } else if (!out) { > + out = e; > + } > + break; > + default: > + continue; > + } > + } > + > + if (!in || !out || !intr) { > + dev_err(&intf->dev, "Miss Endpoints\n"); > + return false; > + } > + > + if ((in->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 1) { > + dev_err(&intf->dev, "Invalid Rx Endpoints\n"); > + return false; > + } > + > + if ((out->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 2) { > + dev_err(&intf->dev, "Invalid Tx Endpoints\n"); > + return false; > + } > + > + if ((intr->desc.bEndpointAddress & USB_ENDPOINT_NUMBER_MASK) != 3) { > + dev_err(&intf->dev, "Invalid interrupt Endpoints\n"); > + return false; > + } > + > + return true; > +} We have a USB core function that does all of the above for you, why not use that instead? Look at usb_find_common_endpoints() and usb_find_common_endpoints_reverse() and at the very least usb_find_bulk_in_endpoint() and related functions. Please don't open-code this type of logic, it's easy to get things wrong. thanks, greg k-h
