[lvc-project] [PATCH] comedi: vmk80xx: fix incomplete endpoint checking
Ian Abbott
abbotti at mev.co.uk
Mon Apr 8 18:25:39 MSK 2024
On 07/04/2024 17:26, Nikita Zhandarovich wrote:
> While vmk80xx does have endpoint checking implemented, some things
> can fall through the cracks. Depending on the hardware model,
> URBs can have either bulk or interrupt type, and current version
> of vmk80xx_find_usb_endpoints() function does not take that fully
> into account. While this warning does not seem to be too harmful,
> at the very least it will crash systems with 'panic_on_warn' set on
> them.
>
> Fix the issue found by Syzkaller [1] by somewhat simplifying the
> endpoint checking process with usb_find_common_endpoints() and
> ensuring that only expected endpoint types are present.
>
> This patch has not been tested on real hardware.
>
> [1] Syzkaller report:
> usb 1-1: BOGUS urb xfer, pipe 1 != type 3
> WARNING: CPU: 0 PID: 781 at drivers/usb/core/urb.c:504 usb_submit_urb+0xc4e/0x18c0 drivers/usb/core/urb.c:503
> ...
> Call Trace:
> <TASK>
> usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
> vmk80xx_reset_device drivers/comedi/drivers/vmk80xx.c:227 [inline]
> vmk80xx_auto_attach+0xa1c/0x1a40 drivers/comedi/drivers/vmk80xx.c:818
> comedi_auto_config+0x238/0x380 drivers/comedi/drivers.c:1067
> usb_probe_interface+0x5cd/0xb00 drivers/usb/core/driver.c:399
> ...
>
> Similar issue also found by Syzkaller:
> Link: https://syzkaller.appspot.com/bug?extid=5205eb2f17de3e01946e
>
> Reported-and-tested-by: syzbot+5f29dc6a889fc42bd896 at syzkaller.appspotmail.com
> Fixes: 49253d542cc0 ("staging: comedi: vmk80xx: factor out usb endpoint detection")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich at fintech.ru>
> ---
> drivers/comedi/drivers/vmk80xx.c | 35 ++++++++++++-----------------------
> 1 file changed, 12 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/comedi/drivers/vmk80xx.c b/drivers/comedi/drivers/vmk80xx.c
> index 4536ed43f65b..476885403c61 100644
> --- a/drivers/comedi/drivers/vmk80xx.c
> +++ b/drivers/comedi/drivers/vmk80xx.c
> @@ -641,33 +641,22 @@ static int vmk80xx_find_usb_endpoints(struct comedi_device *dev)
> struct vmk80xx_private *devpriv = dev->private;
> struct usb_interface *intf = comedi_to_usb_interface(dev);
> struct usb_host_interface *iface_desc = intf->cur_altsetting;
> - struct usb_endpoint_descriptor *ep_desc;
> - int i;
> + struct usb_endpoint_descriptor *ep_rx_desc, *ep_tx_desc;
> + int i, ret;
I get a "warning: unused variable 'i' [-Wunused-variable]" warning here.
>
> - if (iface_desc->desc.bNumEndpoints != 2)
> - return -ENODEV;
> -
> - for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
> - ep_desc = &iface_desc->endpoint[i].desc;
> -
> - if (usb_endpoint_is_int_in(ep_desc) ||
> - usb_endpoint_is_bulk_in(ep_desc)) {
> - if (!devpriv->ep_rx)
> - devpriv->ep_rx = ep_desc;
> - continue;
> - }
> -
> - if (usb_endpoint_is_int_out(ep_desc) ||
> - usb_endpoint_is_bulk_out(ep_desc)) {
> - if (!devpriv->ep_tx)
> - devpriv->ep_tx = ep_desc;
> - continue;
> - }
> - }
> + if (devpriv->model == VMK8061_MODEL)
> + ret = usb_find_common_endpoints(iface_desc, &ep_rx_desc,
> + &ep_tx_desc, NULL, NULL);
> + else
> + ret = usb_find_common_endpoints(iface_desc, NULL, NULL,
> + &ep_rx_desc, &ep_tx_desc);
>
> - if (!devpriv->ep_rx || !devpriv->ep_tx)
> + if (ret)
> return -ENODEV;
>
> + devpriv->ep_rx = ep_rx_desc;
> + devpriv->ep_tx = ep_tx_desc;
> +
> if (!usb_endpoint_maxp(devpriv->ep_rx) || !usb_endpoint_maxp(devpriv->ep_tx))
> return -EINVAL;
>
I've tested it on a K8055/VM110 board and it still works OK. I don't
have a K8061/VM140 to test it with, but it should be OK.
Feel free to add "Reviewed-by: Ian Abbott <abbotti at mev.co.uk>" after
fixing the compiler warning.
--
-=( Ian Abbott <abbotti at mev.co.uk> || MEV Ltd. is a company )=-
-=( registered in England & Wales. Regd. number: 02862268. )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
More information about the lvc-project
mailing list