From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9415CCA483 for ; Fri, 8 Jul 2022 11:57:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238218AbiGHL5h (ORCPT ); Fri, 8 Jul 2022 07:57:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52450 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238284AbiGHL5c (ORCPT ); Fri, 8 Jul 2022 07:57:32 -0400 Received: from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com [IPv6:2a00:1450:4864:20::22b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 288019A6B2 for ; Fri, 8 Jul 2022 04:57:31 -0700 (PDT) Received: by mail-lj1-x22b.google.com with SMTP id y18so14785999ljj.6 for ; Fri, 08 Jul 2022 04:57:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kvaser.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=7Oo+gw7kEUwWjH9Szfyb4gfYmnZ/omxWNnxEr9B/70g=; b=UYuAcpHPE/AjPTDrnpFHFyPlcf7wNDdvbeJprsbXRKMDBL6P3qr1kWkbB/pyWzunC9 6eXZYUH8h1uZa4LBm9zOnblqWLRhWEtKoM6Ek1SCn4dn8HM51dXnpFHlSzb+s7Vy5htr ZE49WN0qXh4wtDh27ACrAMtM+i2DRfqCcCsofgZZNHMXD/rlpqlizD/r9pzVuKAyK9IG WIAfc2zB/HXPtB5EbAlM64F9rEbyn1Cc/6C0k3RF7dsnKtysUVH48s2LTAZECA/G5ykS z9RTLWdArHK/7tnThWKtcDl53qlipFKsP0WTqHA3Y401QDB2FwNseKLEXKJmgzzI1laC yBPw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=7Oo+gw7kEUwWjH9Szfyb4gfYmnZ/omxWNnxEr9B/70g=; b=8PShUclpcoklDpprqZ6WIbmjKH2NiVbNYbIKkmY+oH73nCGzMmCQIVxrPMzIZl0Ccp 2AMSb3O72QOrrtCo/yM6+0qlec6HpccUqqI4MpgCwb0DHijySXTWTF3Qn+mFqbOLoF3E jdErYUjXZel+rFCgbXsmmCK8vV5viY1OeG0lkv/PKBOvc69FZ0tHJeLKv56bYi5oN+Nf U+BsQ5KSPr6Hpw3BowfnKChJfvWkmCpWeb3KLc4PByo8no9RFqSHMhgZCJmPmhwWCjVJ SzY44Xl5cqNlWrGNt1FBuO3t0Ckfxto/Lj60qDVthgniycGfu12VisXqiOti4Uf531Gz mWMw== X-Gm-Message-State: AJIora9/JUQCOjSuyu+4IPq3fA+pqOleHmg8k763NLQMV2T03N51FDkE g1af6ijFan+7Wdhdp7EIXHz7aNo7eQud1A== X-Google-Smtp-Source: AGRyM1vhsh/5t/1NKmoTxKBSyVwxAstdAqcGywHLYfRJNZGL3t8ZMoURHH1F4M0LEqoejxQHw1uTTg== X-Received: by 2002:a2e:2e18:0:b0:25d:521d:529c with SMTP id u24-20020a2e2e18000000b0025d521d529cmr1847636lju.352.1657281449503; Fri, 08 Jul 2022 04:57:29 -0700 (PDT) Received: from freke.kvaser.se (rota.kvaser.com. [195.22.86.90]) by smtp.gmail.com with ESMTPSA id h7-20020a2e9ec7000000b0025a724f2935sm7336199ljk.137.2022.07.08.04.57.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jul 2022 04:57:29 -0700 (PDT) From: Jimmy Assarsson To: linux-can@vger.kernel.org, Anssi Hannula Cc: Jimmy Assarsson , Jimmy Assarsson Subject: [PATCH v2 01/15] can: kvaser_usb_leaf: Fix overread with an invalid command Date: Fri, 8 Jul 2022 13:56:55 +0200 Message-Id: <20220708115709.232815-2-extja@kvaser.com> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20220708115709.232815-1-extja@kvaser.com> References: <20220708115709.232815-1-extja@kvaser.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-can@vger.kernel.org From: Anssi Hannula For command events read from the device, kvaser_usb_leaf_read_bulk_callback() verifies that cmd->len does not exceed the size of the received data, but the actual kvaser_cmd handlers will happily read any kvaser_cmd fields without checking for cmd->len. This can cause an overread if the last cmd in the buffer is shorter than expected for the command type (with cmd->len showing the actual short size). Maximum overread seems to be 22 bytes (CMD_LEAF_LOG_MESSAGE), some of which are delivered to userspace as-is. Fix that by verifying the length of command before handling it. This issue can only occur after RX URBs have been set up, i.e. the interface has been opened at least once. Fixes: 080f40a6fa28 ("can: kvaser_usb: Add support for Kvaser CAN/USB devices") Tested-by: Jimmy Assarsson Signed-off-by: Anssi Hannula --- Changes in v2: - Rebased on b3b6df2c56d8 ("can: kvaser_usb: kvaser_usb_leaf: fix bittiming limits") .../net/can/usb/kvaser_usb/kvaser_usb_leaf.c | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) 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 cc809ecd1e62..abe9c75e0fd2 100644 --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_leaf.c @@ -310,6 +310,38 @@ struct kvaser_cmd { } u; } __packed; +#define CMD_SIZE_ANY 0xff +#define kvaser_fsize(field) sizeof_field(struct kvaser_cmd, field) + +static const u8 kvaser_usb_leaf_cmd_sizes_leaf[] = { + [CMD_START_CHIP_REPLY] = kvaser_fsize(u.simple), + [CMD_STOP_CHIP_REPLY] = kvaser_fsize(u.simple), + [CMD_GET_CARD_INFO_REPLY] = kvaser_fsize(u.cardinfo), + [CMD_TX_ACKNOWLEDGE] = kvaser_fsize(u.tx_acknowledge_header), + [CMD_GET_SOFTWARE_INFO_REPLY] = kvaser_fsize(u.leaf.softinfo), + [CMD_RX_STD_MESSAGE] = kvaser_fsize(u.leaf.rx_can), + [CMD_RX_EXT_MESSAGE] = kvaser_fsize(u.leaf.rx_can), + [CMD_LEAF_LOG_MESSAGE] = kvaser_fsize(u.leaf.log_message), + [CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.leaf.chip_state_event), + [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.leaf.error_event), + /* ignored events: */ + [CMD_FLUSH_QUEUE_REPLY] = CMD_SIZE_ANY, +}; + +static const u8 kvaser_usb_leaf_cmd_sizes_usbcan[] = { + [CMD_START_CHIP_REPLY] = kvaser_fsize(u.simple), + [CMD_STOP_CHIP_REPLY] = kvaser_fsize(u.simple), + [CMD_GET_CARD_INFO_REPLY] = kvaser_fsize(u.cardinfo), + [CMD_TX_ACKNOWLEDGE] = kvaser_fsize(u.tx_acknowledge_header), + [CMD_GET_SOFTWARE_INFO_REPLY] = kvaser_fsize(u.usbcan.softinfo), + [CMD_RX_STD_MESSAGE] = kvaser_fsize(u.usbcan.rx_can), + [CMD_RX_EXT_MESSAGE] = kvaser_fsize(u.usbcan.rx_can), + [CMD_CHIP_STATE_EVENT] = kvaser_fsize(u.usbcan.chip_state_event), + [CMD_CAN_ERROR_EVENT] = kvaser_fsize(u.usbcan.error_event), + /* ignored events: */ + [CMD_USBCAN_CLOCK_OVERFLOW_EVENT] = CMD_SIZE_ANY, +}; + /* Summary of a kvaser error event, for a unified Leaf/Usbcan error * handling. Some discrepancies between the two families exist: * @@ -397,6 +429,43 @@ static const struct kvaser_usb_dev_cfg kvaser_usb_leaf_imx_dev_cfg_32mhz = { .bittiming_const = &kvaser_usb_flexc_bittiming_const, }; +static int kvaser_usb_leaf_verify_size(const struct kvaser_usb *dev, + const struct kvaser_cmd *cmd) +{ + /* buffer size >= cmd->len ensured by caller */ + u8 min_size = 0; + + switch (dev->driver_info->family) { + case KVASER_LEAF: + if (cmd->id < ARRAY_SIZE(kvaser_usb_leaf_cmd_sizes_leaf)) + min_size = kvaser_usb_leaf_cmd_sizes_leaf[cmd->id]; + break; + case KVASER_USBCAN: + if (cmd->id < ARRAY_SIZE(kvaser_usb_leaf_cmd_sizes_usbcan)) + min_size = kvaser_usb_leaf_cmd_sizes_usbcan[cmd->id]; + break; + } + + if (min_size == CMD_SIZE_ANY) + return 0; + + if (min_size) { + min_size += CMD_HEADER_LEN; + if (cmd->len >= min_size) + return 0; + + dev_err_ratelimited(&dev->intf->dev, + "Received command %u too short (size %u, needed %u)", + cmd->id, cmd->len, min_size); + return -EIO; + } + + dev_warn_ratelimited(&dev->intf->dev, + "Unhandled command (%d, size %d)\n", + cmd->id, cmd->len); + return -EINVAL; +} + static void * kvaser_usb_leaf_frame_to_cmd(const struct kvaser_usb_net_priv *priv, const struct sk_buff *skb, int *cmd_len, @@ -502,6 +571,9 @@ static int kvaser_usb_leaf_wait_cmd(const struct kvaser_usb *dev, u8 id, end: kfree(buf); + if (err == 0) + err = kvaser_usb_leaf_verify_size(dev, cmd); + return err; } @@ -1130,6 +1202,9 @@ static void kvaser_usb_leaf_stop_chip_reply(const struct kvaser_usb *dev, static void kvaser_usb_leaf_handle_command(const struct kvaser_usb *dev, const struct kvaser_cmd *cmd) { + if (kvaser_usb_leaf_verify_size(dev, cmd) < 0) + return; + switch (cmd->id) { case CMD_START_CHIP_REPLY: kvaser_usb_leaf_start_chip_reply(dev, cmd); -- 2.36.1