From patchwork Fri Oct 18 13:44:14 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juerg Haefliger X-Patchwork-Id: 1999172 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XVQtK4cRqz1xw2 for ; Sat, 19 Oct 2024 00:44:41 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1t1nHN-00071G-4c; Fri, 18 Oct 2024 13:44:33 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1t1nHK-0006w1-M6 for kernel-team@lists.ubuntu.com; Fri, 18 Oct 2024 13:44:30 +0000 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 981D13F133 for ; Fri, 18 Oct 2024 13:44:23 +0000 (UTC) Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-4315af466d9so12479145e9.3 for ; Fri, 18 Oct 2024 06:44:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729259062; x=1729863862; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=4nnCQTRaKY2BojEANbSTpBpGk5Q8oVFBWF5JBRGSdd4=; b=hqbzCB3gK0tQ/idel0xT+MXfUTHLQOTVQO1Vs5t9+3vXOfExC0ICWREG6exKSNUZyG i2rALuupifD/qwUAS6rhNRlsTEuIgI2I2nIC/sV5PWvAhbr9RYvn2jv0nkshO3GOI+t3 DUuLmMxFwQNAjKp0TeU5vLrkDaIGkbm1qWiS6lsagoDQijsW8QsJlGvWERe8d/E8wdjq VEBUGPbMfdhb9ilvEv6mK51yfcEZE3JvC1sZMj2YGTU/g+uUncsRY9SpLkV6gPYSZW+Y Pj8Bcet5/fqjNNQ+sCalAYE4vu//jygTGRleNB36tqOcQkmwsxiH/CHYsvgeKKSTitXZ aqNg== X-Gm-Message-State: AOJu0Ywiuj1DWW8t444IpWeVQ1n81sCMN1G8cbbDv2vrnYeaKesAA8Z7 qZMCg/AxBbliIekJdDr3zZBAPFQ9JjGfnNz0J2peEFNg7dtSpMG/uXDR/Zk0QMmUXUzItEqNO9J lPmpLfQpkl4qcR4Yd3Nd11xbAb17ZXWQXO/LxjonCBYCPNVq8LtFSH3PaxMS2rgtQiIH2YiqsZc u2IFOlDvY+Mg== X-Received: by 2002:a05:600c:3592:b0:431:586e:7e7 with SMTP id 5b1f17b1804b1-43161634f28mr19030505e9.1.1729259061871; Fri, 18 Oct 2024 06:44:21 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFtY8WAOXkcanoNVjRJVGZG+ZA50DSWn+MQy/4/U4ahmt6v7bfL7qG572tWUg4Dzag/fs5J3A== X-Received: by 2002:a05:600c:3592:b0:431:586e:7e7 with SMTP id 5b1f17b1804b1-43161634f28mr19030275e9.1.1729259061324; Fri, 18 Oct 2024 06:44:21 -0700 (PDT) Received: from localhost ([81.221.247.52]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-43160dbe77dsm24871645e9.8.2024.10.18.06.44.20 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Oct 2024 06:44:20 -0700 (PDT) From: Juerg Haefliger To: kernel-team@lists.ubuntu.com Subject: [SRU][F][PATCH 1/1] Bluetooth: L2CAP: Fix div-by-zero in l2cap_le_flowctl_init() Date: Fri, 18 Oct 2024 15:44:14 +0200 Message-ID: X-Mailer: git-send-email 2.43.0 In-Reply-To: References: MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Sungwoo Kim l2cap_le_flowctl_init() can cause both div-by-zero and an integer overflow since hdev->le_mtu may not fall in the valid range. Move MTU from hci_dev to hci_conn to validate MTU and stop the connection process earlier if MTU is invalid. Also, add a missing validation in read_buffer_size() and make it return an error value if the validation fails. Now hci_conn_add() returns ERR_PTR() as it can fail due to the both a kzalloc failure and invalid MTU value. divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI CPU: 0 PID: 67 Comm: kworker/u5:0 Tainted: G W 6.9.0-rc5+ #20 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 Workqueue: hci0 hci_rx_work RIP: 0010:l2cap_le_flowctl_init+0x19e/0x3f0 net/bluetooth/l2cap_core.c:547 Code: e8 17 17 0c 00 66 41 89 9f 84 00 00 00 bf 01 00 00 00 41 b8 02 00 00 00 4c 89 fe 4c 89 e2 89 d9 e8 27 17 0c 00 44 89 f0 31 d2 <66> f7 f3 89 c3 ff c3 4d 8d b7 88 00 00 00 4c 89 f0 48 c1 e8 03 42 RSP: 0018:ffff88810bc0f858 EFLAGS: 00010246 RAX: 00000000000002a0 RBX: 0000000000000000 RCX: dffffc0000000000 RDX: 0000000000000000 RSI: ffff88810bc0f7c0 RDI: ffffc90002dcb66f RBP: ffff88810bc0f880 R08: aa69db2dda70ff01 R09: 0000ffaaaaaaaaaa R10: 0084000000ffaaaa R11: 0000000000000000 R12: ffff88810d65a084 R13: dffffc0000000000 R14: 00000000000002a0 R15: ffff88810d65a000 FS: 0000000000000000(0000) GS:ffff88811ac00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020000100 CR3: 0000000103268003 CR4: 0000000000770ef0 PKRU: 55555554 Call Trace: l2cap_le_connect_req net/bluetooth/l2cap_core.c:4902 [inline] l2cap_le_sig_cmd net/bluetooth/l2cap_core.c:5420 [inline] l2cap_le_sig_channel net/bluetooth/l2cap_core.c:5486 [inline] l2cap_recv_frame+0xe59d/0x11710 net/bluetooth/l2cap_core.c:6809 l2cap_recv_acldata+0x544/0x10a0 net/bluetooth/l2cap_core.c:7506 hci_acldata_packet net/bluetooth/hci_core.c:3939 [inline] hci_rx_work+0x5e5/0xb20 net/bluetooth/hci_core.c:4176 process_one_work kernel/workqueue.c:3254 [inline] process_scheduled_works+0x90f/0x1530 kernel/workqueue.c:3335 worker_thread+0x926/0xe70 kernel/workqueue.c:3416 kthread+0x2e3/0x380 kernel/kthread.c:388 ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244 Modules linked in: ---[ end trace 0000000000000000 ]--- Fixes: 6ed58ec520ad ("Bluetooth: Use LE buffers for LE traffic") Suggested-by: Luiz Augusto von Dentz Signed-off-by: Sungwoo Kim Signed-off-by: Luiz Augusto von Dentz (backported from commit a5b862c6a221459d54e494e88965b48dcfa6cc44) [juergh: Adjusted context plus: - Add #define HCI_ERROR_INVALID_PARAMETERS to include/net/bluetooth/hci.h, from: c86cc5a3ec70 ("Bluetooth: hci_event: Fix checking for invalid handle on error status") - Drop modifications of net/bluetooth/iso.c and ISO_LINK (not supported in 5.4) - Change return type of hci_cc_read_buffer_size() and hci_cc_le_read_buffer_size() from void to u8 and return status, from: c8992cffbe74 ("Bluetooth: hci_event: Use of a function table to handle Command Complete") - Drop various hunks due to missing functionality in 5.4] CVE-2024-36968 Signed-off-by: Juerg Haefliger --- include/net/bluetooth/hci.h | 10 ++++++++ include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_conn.c | 44 +++++++++++++++++++++++++------- net/bluetooth/hci_event.c | 36 ++++++++++++++++---------- net/bluetooth/l2cap_core.c | 17 +++--------- net/bluetooth/sco.c | 6 ++--- 6 files changed, 75 insertions(+), 39 deletions(-) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 5bc1e30dedde..2a0593d95304 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -496,6 +496,7 @@ enum { #define HCI_ERROR_CONNECTION_TIMEOUT 0x08 #define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d #define HCI_ERROR_REJ_BAD_ADDR 0x0f +#define HCI_ERROR_INVALID_PARAMETERS 0x12 #define HCI_ERROR_REMOTE_USER_TERM 0x13 #define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14 #define HCI_ERROR_REMOTE_POWER_OFF 0x15 @@ -1347,6 +1348,15 @@ struct hci_cp_le_set_event_mask { __u8 mask[8]; } __packed; +/* BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 4, Part E + * 7.8.2 LE Read Buffer Size command + * MAX_LE_MTU is 0xffff. + * 0 is also valid. It means that no dedicated LE Buffer exists. + * It should use the HCI_Read_Buffer_Size command and mtu is shared + * between BR/EDR and LE. + */ +#define HCI_MIN_LE_MTU 0x001b + #define HCI_OP_LE_READ_BUFFER_SIZE 0x2002 struct hci_rp_le_read_buffer_size { __u8 status; diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 6ba83a54dda7..5e7f44971ffc 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -467,6 +467,7 @@ struct hci_conn { __u8 resp_addr_type; __u16 handle; __u16 state; + __u16 mtu; __u8 mode; __u8 type; __u8 role; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index d55973cb5b54..b4384267c922 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -494,11 +494,32 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, { struct hci_conn *conn; + switch (type) { + case ACL_LINK: + if (!hdev->acl_mtu) + return ERR_PTR(-ECONNREFUSED); + break; + case LE_LINK: + if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU) + return ERR_PTR(-ECONNREFUSED); + if (!hdev->le_mtu && hdev->acl_mtu < HCI_MIN_LE_MTU) + return ERR_PTR(-ECONNREFUSED); + break; + case SCO_LINK: + case ESCO_LINK: + if (!hdev->sco_pkts) + /* Controller does not support SCO or eSCO over HCI */ + return ERR_PTR(-ECONNREFUSED); + break; + default: + return ERR_PTR(-ECONNREFUSED); + } + BT_DBG("%s dst %pMR", hdev->name, dst); conn = kzalloc(sizeof(*conn), GFP_KERNEL); if (!conn) - return NULL; + return ERR_PTR(-ENOMEM); bacpy(&conn->dst, dst); bacpy(&conn->src, &hdev->bdaddr); @@ -527,10 +548,12 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, switch (type) { case ACL_LINK: conn->pkt_type = hdev->pkt_type & ACL_PTYPE_MASK; + conn->mtu = hdev->acl_mtu; break; case LE_LINK: /* conn->src should reflect the local identity address */ hci_copy_identity_address(hdev, &conn->src, &conn->src_type); + conn->mtu = hdev->le_mtu ? hdev->le_mtu : hdev->acl_mtu; break; case SCO_LINK: if (lmp_esco_capable(hdev)) @@ -538,9 +561,12 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, (hdev->esco_type & EDR_ESCO_MASK); else conn->pkt_type = hdev->pkt_type & SCO_PTYPE_MASK; + + conn->mtu = hdev->sco_mtu; break; case ESCO_LINK: conn->pkt_type = hdev->esco_type & ~EDR_ESCO_MASK; + conn->mtu = hdev->sco_mtu; break; } @@ -1009,8 +1035,8 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst, bacpy(&conn->dst, dst); } else { conn = hci_conn_add(hdev, LE_LINK, dst, role); - if (!conn) - return ERR_PTR(-ENOMEM); + if (IS_ERR(conn)) + return conn; hci_conn_hold(conn); conn->pending_sec_level = sec_level; } @@ -1171,8 +1197,8 @@ struct hci_conn *hci_connect_le_scan(struct hci_dev *hdev, bdaddr_t *dst, BT_DBG("requesting refresh of dst_addr"); conn = hci_conn_add(hdev, LE_LINK, dst, HCI_ROLE_MASTER); - if (!conn) - return ERR_PTR(-ENOMEM); + if (IS_ERR(conn)) + return conn; if (hci_explicit_conn_params_set(hdev, dst, dst_type) < 0) { hci_conn_del(conn); @@ -1217,8 +1243,8 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst, acl = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst); if (!acl) { acl = hci_conn_add(hdev, ACL_LINK, dst, HCI_ROLE_MASTER); - if (!acl) - return ERR_PTR(-ENOMEM); + if (IS_ERR(acl)) + return acl; } hci_conn_hold(acl); @@ -1246,9 +1272,9 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst, sco = hci_conn_hash_lookup_ba(hdev, type, dst); if (!sco) { sco = hci_conn_add(hdev, type, dst, HCI_ROLE_MASTER); - if (!sco) { + if (IS_ERR(sco)) { hci_conn_drop(acl); - return ERR_PTR(-ENOMEM); + return sco; } } diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 447841c15351..055a4e234ea7 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -721,14 +721,14 @@ static void hci_cc_read_flow_control_mode(struct hci_dev *hdev, hdev->flow_ctl_mode = rp->mode; } -static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb) +static u8 hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb) { struct hci_rp_read_buffer_size *rp = (void *) skb->data; BT_DBG("%s status 0x%2.2x", hdev->name, rp->status); if (rp->status) - return; + return rp->status; hdev->acl_mtu = __le16_to_cpu(rp->acl_mtu); hdev->sco_mtu = rp->sco_mtu; @@ -745,6 +745,11 @@ static void hci_cc_read_buffer_size(struct hci_dev *hdev, struct sk_buff *skb) BT_DBG("%s acl mtu %d:%d sco mtu %d:%d", hdev->name, hdev->acl_mtu, hdev->acl_pkts, hdev->sco_mtu, hdev->sco_pkts); + + if (!hdev->acl_mtu || !hdev->acl_pkts) + return HCI_ERROR_INVALID_PARAMETERS; + + return rp->status; } static void hci_cc_read_bd_addr(struct hci_dev *hdev, struct sk_buff *skb) @@ -961,15 +966,15 @@ static void hci_cc_pin_code_neg_reply(struct hci_dev *hdev, struct sk_buff *skb) hci_dev_unlock(hdev); } -static void hci_cc_le_read_buffer_size(struct hci_dev *hdev, - struct sk_buff *skb) +static u8 hci_cc_le_read_buffer_size(struct hci_dev *hdev, + struct sk_buff *skb) { struct hci_rp_le_read_buffer_size *rp = (void *) skb->data; BT_DBG("%s status 0x%2.2x", hdev->name, rp->status); if (rp->status) - return; + return rp->status; hdev->le_mtu = __le16_to_cpu(rp->le_mtu); hdev->le_pkts = rp->le_max_pkt; @@ -977,6 +982,11 @@ static void hci_cc_le_read_buffer_size(struct hci_dev *hdev, hdev->le_cnt = hdev->le_pkts; BT_DBG("%s le mtu %d:%d", hdev->name, hdev->le_mtu, hdev->le_pkts); + + if (hdev->le_mtu && hdev->le_mtu < HCI_MIN_LE_MTU) + return HCI_ERROR_INVALID_PARAMETERS; + + return rp->status; } static void hci_cc_le_read_local_features(struct hci_dev *hdev, @@ -1819,8 +1829,8 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (!conn) { conn = hci_conn_add(hdev, ACL_LINK, &cp->bdaddr, HCI_ROLE_MASTER); - if (!conn) - bt_dev_err(hdev, "no memory for new connection"); + if (IS_ERR(conn)) + bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn)); } } @@ -2671,8 +2681,8 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) if (!conn) { conn = hci_conn_add(hdev, ev->link_type, &ev->bdaddr, HCI_ROLE_SLAVE); - if (!conn) { - bt_dev_err(hdev, "no memory for new connection"); + if (IS_ERR(conn)) { + bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn)); hci_dev_unlock(hdev); return; } @@ -3297,7 +3307,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb, break; case HCI_OP_READ_BUFFER_SIZE: - hci_cc_read_buffer_size(hdev, skb); + *status = hci_cc_read_buffer_size(hdev, skb); break; case HCI_OP_READ_BD_ADDR: @@ -3357,7 +3367,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb, break; case HCI_OP_LE_READ_BUFFER_SIZE: - hci_cc_le_read_buffer_size(hdev, skb); + *status = hci_cc_le_read_buffer_size(hdev, skb); break; case HCI_OP_LE_READ_LOCAL_FEATURES: @@ -5066,8 +5076,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, conn = hci_lookup_le_connect(hdev); if (!conn) { conn = hci_conn_add(hdev, LE_LINK, bdaddr, role); - if (!conn) { - bt_dev_err(hdev, "no memory for new connection"); + if (IS_ERR(conn)) { + bt_dev_err(hdev, "connection err: %ld", PTR_ERR(conn)); goto unlock; } diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 874f12d93bfa..8387d4183716 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -6572,7 +6572,7 @@ static int l2cap_finish_move(struct l2cap_chan *chan) if (chan->hs_hcon) chan->conn->mtu = chan->hs_hcon->hdev->block_mtu; else - chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu; + chan->conn->mtu = chan->conn->hcon->mtu; return l2cap_resegment(chan); } @@ -6643,7 +6643,7 @@ static int l2cap_rx_state_wait_f(struct l2cap_chan *chan, if (chan->hs_hcon) chan->conn->mtu = chan->hs_hcon->hdev->block_mtu; else - chan->conn->mtu = chan->conn->hcon->hdev->acl_mtu; + chan->conn->mtu = chan->conn->hcon->mtu; err = l2cap_resegment(chan); @@ -7192,18 +7192,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon) BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan); - switch (hcon->type) { - case LE_LINK: - if (hcon->hdev->le_mtu) { - conn->mtu = hcon->hdev->le_mtu; - break; - } - /* fall through */ - default: - conn->mtu = hcon->hdev->acl_mtu; - break; - } - + conn->mtu = hcon->mtu; conn->feat_mask = 0; conn->local_fixed_chan = L2CAP_FC_SIG_BREDR | L2CAP_FC_CONNLESS; diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index 4c81883ba25e..92e0712edf8d 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -126,7 +126,6 @@ static void sco_sock_clear_timer(struct sock *sk) /* ---- SCO connections ---- */ static struct sco_conn *sco_conn_add(struct hci_conn *hcon) { - struct hci_dev *hdev = hcon->hdev; struct sco_conn *conn = hcon->sco_data; if (conn) @@ -141,9 +140,10 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon) hcon->sco_data = conn; conn->hcon = hcon; + conn->mtu = hcon->mtu; - if (hdev->sco_mtu > 0) - conn->mtu = hdev->sco_mtu; + if (hcon->mtu > 0) + conn->mtu = hcon->mtu; else conn->mtu = 60;