Messages in this thread | | | Date | Sat, 14 May 2022 15:24:53 +0200 | From | Greg KH <> | Subject | Re: [PATCH v2] char: xillybus: Check endpoint type before allocing |
| |
On Sat, May 14, 2022 at 03:52:20PM +0300, Eli Billauer wrote: > On 14/05/22 14:48, Zheyu Ma wrote: > > The driver submits bulk urb without checking the endpoint type is > > actually bulk. > > > > [ 3.108690] usb 1-1: BOGUS urb xfer, pipe 3 != type 1 > > [ 3.108983] WARNING: CPU: 0 PID: 211 at drivers/usb/core/urb.c:503 usb_submit_urb+0xcd9/0x18b0 > > [ 3.110976] RIP: 0010:usb_submit_urb+0xcd9/0x18b0 > > [ 3.115318] Call Trace: > > [ 3.115452]<TASK> > > [ 3.115570] try_queue_bulk_in+0x43c/0x6e0 [xillyusb] > > [ 3.115838] xillyusb_probe+0x488/0x1230 [xillyusb] > > > > Add a check in endpoint_alloc() to fix the bug. > > > > Signed-off-by: Zheyu Ma<zheyuma97@gmail.com> > > --- > > Changes in v2: > > - Check the endpoint type at probe time > > --- > > drivers/char/xillybus/xillyusb.c | 27 ++++++++++++++++++++++++++- > > 1 file changed, 26 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/char/xillybus/xillyusb.c b/drivers/char/xillybus/xillyusb.c > > index dc3551796e5e..4467f13993ef 100644 > > --- a/drivers/char/xillybus/xillyusb.c > > +++ b/drivers/char/xillybus/xillyusb.c > > @@ -167,6 +167,7 @@ struct xillyusb_dev { > > struct device *dev; /* For dev_err() and such */ > > struct kref kref; > > struct workqueue_struct *workq; > > + struct usb_interface *intf; > > > > int error; > > spinlock_t error_lock; /* protect @error */ > > @@ -475,6 +476,25 @@ static void endpoint_dealloc(struct xillyusb_endpoint *ep) > > kfree(ep); > > } > > > > +static int xillyusb_check_endpoint(struct xillyusb_dev *xdev, u8 ep_num) > > +{ > > + struct usb_host_interface *if_desc = xdev->intf->altsetting; > > + int i; > > + > > + for (i = 0; i< if_desc->desc.bNumEndpoints; i++) { > > + struct usb_endpoint_descriptor *ep =&if_desc->endpoint[i].desc; > > + > > + if (ep->bEndpointAddress != ep_num) > > + continue; > > + > > + if ((usb_pipein(ep_num)&& usb_endpoint_is_bulk_in(ep)) || > > + (usb_pipeout(ep_num)&& usb_endpoint_is_bulk_out(ep))) > > + return 0; > > + } > > + > > + return -EINVAL; > > +} > > + > > static struct xillyusb_endpoint > > *endpoint_alloc(struct xillyusb_dev *xdev, > > u8 ep_num, > > @@ -482,10 +502,14 @@ static struct xillyusb_endpoint > > unsigned int order, > > int bufnum) > > { > > - int i; > > + int i, ret; > > > > struct xillyusb_endpoint *ep; > > > > + ret = xillyusb_check_endpoint(xdev, ep_num); > > + if (ret) > > + return NULL; > > + > > ep = kzalloc(sizeof(*ep), GFP_KERNEL); > > > > if (!ep) > > @@ -2125,6 +2149,7 @@ static int xillyusb_probe(struct usb_interface *interface, > > mutex_init(&xdev->process_in_mutex); > > mutex_init(&xdev->msg_mutex); > > > > + xdev->intf = interface; > > xdev->udev = usb_get_dev(interface_to_usbdev(interface)); > > xdev->dev =&interface->dev; > > xdev->error = 0; > I wonder why this check is necessary. The XillyUSB device presents bulk > endpoints only, and the driver never tries anything else.
And the driver needs to check this to verify it. Think about a "fake" device, you have to catch that.
thanks,
greg k-h
| |