Re: [PATCH] can: kvaser_usb: fix potential crash in probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Divide by Zero / Denial of Service in TX acknowledge handler.
- [High] Information leak of uninitialized kernel heap memory to USB device.
--

commit 75516ad10c900aec22101c30d8bc4a80c7eebdbf
Author: Anton Olsson <anol@kvaser.com>
Subject: can: kvaser_usb: fix potential crash in probe

The leaf driver does not return any errors when no clock configuration can
be found, instead emitting a null-check warning. This patch adds default
cases to return -ENODEV when no matching configuration is found.

Link: https://lore.kernel.org/linux-can/20260828-sassy-ammonite-of-focus-b2f17c-mkl@pengutronix.de/T/#t

> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> index a876c7819b81c..2091de2e4f8fc 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c
> @@ -747,8 +747,8 @@ static int kvaser_usb_leaf_send_simple_cmd(const struct kvaser_usb *dev,
>  	return rc;
>  }
>  
> -static void kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
> -						   const struct leaf_cmd_softinfo *softinfo)
> +static int kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
> +						  const struct leaf_cmd_softinfo *softinfo)
>  {
>  	u32 fw_version;
>  	u32 sw_options = le32_to_cpu(softinfo->sw_options);

[Severity: Critical]
This is a pre-existing issue, but further down in this function,
softinfo->max_outstanding_tx is read from the device and stored without
validation:

	dev->max_tx_urbs = le16_to_cpu(softinfo->max_outstanding_tx);

If a malicious USB device reports 0 during initialization, could this trigger
a kernel panic due to a divide-by-zero when the USB device later sends a TX
acknowledge packet? For instance, in kvaser_usb_leaf_tx_acknowledge():

	context = &priv->tx_contexts[tid % dev->max_tx_urbs];

The Hydra device family seems to be subject to the exact same vulnerability
in kvaser_usb_hydra_tx_acknowledge() via transid % dev->max_tx_urbs.

Should we validate that dev->max_tx_urbs is greater than 0 before continuing?

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed another potential problem in
kvaser_usb_leaf_frame_to_cmd() where a command structure is allocated using
kmalloc_obj() without zeroing the memory:

drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c:kvaser_usb_leaf_frame_to_cmd() {
	...
	cmd = kmalloc_obj(*cmd, GFP_ATOMIC);
	if (cmd) {
	...
}

For standard CAN frames, the driver initializes data[0] and data[1], but
appears to leave data[2..4] uninitialized:

		} else {
			cmd->id = CMD_TX_STD_MESSAGE;
			cmd->u.tx_can.data[0] = (cf->can_id >> 6) & 0x1f;
			cmd->u.tx_can.data[1] = cf->can_id & 0x3f;
		}

Additionally, if cf->len is less than 8, the remainder of the 14-byte data
array up to data[13] seems to remain uninitialized:

		cmd->u.tx_can.data[5] = can_get_cc_dlc(cf, priv->can.ctrlmode);
		memcpy(&cmd->u.tx_can.data[6], cf->data, cf->len);

Since the entire structure is then transmitted over the USB bus, can this
leak uninitialized kernel heap memory to an attached USB device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902110611.2426535-1-anol@kvaser.com?part=1