Re: [PATCH net-next 31/37] can: kvaser_usb: refactor endpoint lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: kvaser_usb: infinite loop when bulk-in wMaxPacketSize is 0
--

commit b4168d21a894a8ed379ae53431fcd63f4de542bb
Author: Anton Olsson <anol@kvaser.com>

can: kvaser_usb: refactor endpoint lookup

This patch moves kvaser_usb_leaf_setup_endpoints() to kvaser_usb_core()
so it can be used by both the mhydra and leaf sub-drivers. It also
removes dev_setup_endpoint() from struct kvaser_usb_dev_ops.

Link: https://patch.msgid.link/20260817112052.1477126-1-anol@kvaser.com

> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
> index d0a2a2a33c1c8..9687dfb89b4cc 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
[ ... ]
> @@ -926,6 +926,23 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev, int channel)
>  	return err;
>  }
>  
> +static int kvaser_usb_setup_endpoints(struct kvaser_usb *dev)
> +{
> +	struct usb_host_interface *iface_desc;
> +	int ret;
> +
> +	iface_desc = dev->intf->cur_altsetting;
> +
> +	/* use first bulk endpoint for in and out */
> +	ret = usb_find_common_endpoints(iface_desc, &dev->bulk_in,
> +					&dev->bulk_out, NULL, NULL);
> +
> +	if (ret)
> +		return -ENODEV;
> +

[Severity: High]
Does this code need to validate that dev->bulk_in->wMaxPacketSize != 0?

If a malicious USB device presents a bulk-in endpoint with a wMaxPacketSize
of 0, the USB core does not reject it. Later, in
kvaser_usb_leaf_read_bulk_callback() and kvaser_usb_leaf_wait_cmd(), if a
zero-length command is received (cmd->len == 0), the driver advances the
position using:

    pos = round_up(pos + 1, le16_to_cpu(dev->bulk_in->wMaxPacketSize));

When wMaxPacketSize is 0, round_up(pos + 1, 0) evaluates to 0 because
__round_mask(x, 0) becomes -1. This endlessly resets pos to 0, resulting in
an infinite loop and a CPU hang.

Could we add a check here during endpoint initialization to reject endpoints
with a zero wMaxPacketSize?

> +	return 0;
> +}
> +
>  static int kvaser_usb_probe(struct usb_interface *intf,
>  			    const struct usb_device_id *id)
>  {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009134323.64064-1-mkl@pengutronix.de?part=31