[PATCH net] can: kvaser_usb: validate command format before parsing in hydra receive path

"Cen Zhang (Microsoft)" <[email protected]>
Newsgroups org.kernel.vger.linux-can,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
The receive-path command parsers (kvaser_usb_hydra_wait_cmd and
kvaser_usb_hydra_read_bulk_callback) call kvaser_usb_hydra_cmd_size()
without verifying that enough buffer remains.  For CMD_EXTENDED,
kvaser_usb_hydra_cmd_size() unconditionally reads a 2-byte len field
at offset 4 (kvaser_usb_hydra.c:532).  A malicious USB device can place
a CMD_EXTENDED header at the end of a 3072-byte bulk transfer such that
only 4 bytes remain, causing a 2-byte slab-out-of-bounds read.

  BUG: KASAN: slab-out-of-bounds in kvaser_usb_hydra_wait_cmd+0x3f1/0x480
    [kvaser_usb_hydra.c:678]
  Read of size 2 at addr ffff888013f7ec00 by task kworker/0:0/9
   kvaser_usb_hydra_wait_cmd+0x3f1/0x480
   kvaser_usb_hydra_get_software_details+0x1c7/0x5d0
   kvaser_usb_probe+0x36a/0x1240

Additionally, if the device sends CMD_EXTENDED with len=0,
kvaser_usb_hydra_cmd_size() returns 0 and the parser loops forever
(pos += 0), permanently burning one CPU core.

Fix by adding kvaser_usb_hydra_cmd_size_rx(), a receive-path wrapper that
validates buffer bounds before calling kvaser_usb_hydra_cmd_size().
Callers check for zero return to reject both issues above.

Fixes: aec5fb2268b7 ("can: kvaser_usb: Add support for Kvaser USB hydra family")
Reported-by: [email protected]
Reported-by: Xiang Mei (Microsoft) <[email protected]>
Reported-by: Cen Zhang (Microsoft) <[email protected]>
Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
---
The zero-length command loop is also addressed by:
https://lore.kernel.org/linux-can/[email protected]/

This patch additionally handles truncated command headers in the
synchronous wait and asynchronous receive paths, including the
leftover-buffer path.

 .../net/can/usb/kvaser_usb/kvaser_usb_hydra.c | 34 ++++++++++++++++---
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
index efbb7bed34c9..a964edde5783 100644
--- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
+++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_hydra.c
@@ -536,6 +536,20 @@ static size_t kvaser_usb_hydra_cmd_size(struct kvaser_cmd *cmd)
 	return ret;
 }
 
+/* Receive-path wrapper: validate buffer bounds before reading cmd_size. */
+static size_t kvaser_usb_hydra_cmd_size_rx(struct kvaser_cmd *cmd,
+					   size_t remaining)
+{
+	if (remaining < sizeof(struct kvaser_cmd_header))
+		return 0;
+
+	if (cmd->header.cmd_no == CMD_EXTENDED &&
+	    remaining < offsetof(struct kvaser_cmd_ext, cmd_no_ext))
+		return 0;
+
+	return kvaser_usb_hydra_cmd_size(cmd);
+}
+
 static struct kvaser_usb_net_priv *
 kvaser_usb_hydra_net_priv_from_cmd(const struct kvaser_usb *dev,
 				   const struct kvaser_cmd *cmd)
@@ -675,8 +689,9 @@ static int kvaser_usb_hydra_wait_cmd(const struct kvaser_usb *dev, u8 cmd_no,
 			size_t cmd_len;
 
 			tmp_cmd = buf + pos;
-			cmd_len = kvaser_usb_hydra_cmd_size(tmp_cmd);
-			if (pos + cmd_len > actual_len) {
+			cmd_len = kvaser_usb_hydra_cmd_size_rx(tmp_cmd,
+							       actual_len - pos);
+			if (!cmd_len || pos + cmd_len > actual_len) {
 				dev_err_ratelimited(&dev->intf->dev,
 						    "Format error\n");
 				break;
@@ -2124,7 +2139,14 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 
 		cmd = (struct kvaser_cmd *)card_data->usb_rx_leftover;
 
-		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+		cmd_len = kvaser_usb_hydra_cmd_size_rx(cmd,
+						       KVASER_USB_HYDRA_MAX_CMD_LEN);
+
+		if (!cmd_len) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			spin_unlock_irqrestore(usb_rx_leftover_lock, irq_flags);
+			return;
+		}
 
 		remaining_bytes = min_t(unsigned int, len,
 					cmd_len - usb_rx_leftover_len);
@@ -2154,7 +2176,11 @@ static void kvaser_usb_hydra_read_bulk_callback(struct kvaser_usb *dev,
 	while (pos < len) {
 		cmd = buf + pos;
 
-		cmd_len = kvaser_usb_hydra_cmd_size(cmd);
+		cmd_len = kvaser_usb_hydra_cmd_size_rx(cmd, len - pos);
+		if (!cmd_len) {
+			dev_err(&dev->intf->dev, "Format error\n");
+			return;
+		}
 
 		if (pos + cmd_len > len) {
 			/* We got first part of a command */
-- 
2.52.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.