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

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

[Anton Olsson]

The leaf driver does not return any errors when no clock configuration
can be found, and instead it emits a null-check warning. Add default
cases to return an error when no matching clock configuration can be
found.

Signed-off-by: Anton Olsson <anol@kvaser.com>
Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Link: https://lore.kernel.org/linux-can/20260828-sassy-ammonite-of-focus-b2f17c-mkl@pengutronix.de/T/#t
Reviewed-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

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 a876c7819b81..2091de2e4f8f 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);
@@ -777,6 +777,8 @@ static void kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
 		case KVASER_USB_LEAF_SWOPTION_FREQ_32_MHZ_CLK:
 			dev->cfg = &kvaser_usb_leaf_m32c_dev_cfg_32mhz;
 			break;
+		default:
+			return -ENODEV;
 		}
 	} else {
 		switch (sw_options & KVASER_USB_LEAF_SWOPTION_FREQ_MASK) {
@@ -789,8 +791,11 @@ static void kvaser_usb_leaf_get_software_info_leaf(struct kvaser_usb *dev,
 		case KVASER_USB_LEAF_SWOPTION_FREQ_32_MHZ_CLK:
 			dev->cfg = &kvaser_usb_leaf_imx_dev_cfg_32mhz;
 			break;
+		default:
+			return -ENODEV;
 		}
 	}
+	return 0;
 }
 
 static int kvaser_usb_leaf_get_software_info_inner(struct kvaser_usb *dev)
@@ -809,7 +814,9 @@ static int kvaser_usb_leaf_get_software_info_inner(struct kvaser_usb *dev)
 
 	switch (dev->driver_info->family) {
 	case KVASER_LEAF:
-		kvaser_usb_leaf_get_software_info_leaf(dev, &cmd.u.leaf.softinfo);
+		err = kvaser_usb_leaf_get_software_info_leaf(dev, &cmd.u.leaf.softinfo);
+		if (err)
+			return err;
 		break;
 	case KVASER_USBCAN:
 		fw_version = le32_to_cpu(cmd.u.usbcan.softinfo.fw_version);
-- 
2.43.0

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

[sashiko-bot]

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