lkml.org 
[lkml]   [2022]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-05-14 15:26    [W:0.870 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site