netdev - [RFC PATCH] Bluetooth: hci_event: Ignore multiple conn complete events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220121173622.192744-1-soenke.huster@eknoes.de>
Date:   Fri, 21 Jan 2022 18:36:22 +0100
From:   Soenke Huster <soenke.huster@...oes.de>
To:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Soenke Huster <soenke.huster@...oes.de>,
        linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [RFC PATCH] Bluetooth: hci_event: Ignore multiple conn complete events

When a HCI_CONNECTION_COMPLETE event is received multiple times
for the same handle, the device is registered multiple times which leads
to memory corruptions. Therefore, consequent events for a single
connection are ignored.

The conn->state can hold different values so conn->handle is
checked to detect whether a connection is already set up.

Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=215497
Signed-off-by: Soenke Huster <soenke.huster@...oes.de>
---
This fixes the referenced bug and several use-after-free issues I discovered.
I tagged it as RFC, as I am not 100% sure if checking the existence of the
handle is the correct approach, but to the best of my knowledge it must be
set for the first time in this function for valid connections of this event,
therefore it should be fine.

net/bluetooth/hci_event.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 681c623aa380..71ccb12c928d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -3106,6 +3106,17 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
 		}
 	}
 
+	/* The HCI_Connection_Complete event is only sent once per connection.
+	 * Processing it more than once per connection can corrupt kernel memory.
+	 *
+	 * As the connection handle is set here for the first time, it indicates
+	 * whether the connection is already set up.
+	 */
+	if (conn->handle) {
+		bt_dev_err(hdev, "Ignoring HCI_Connection_Complete for existing connection");
+		goto unlock;
+	}
+
 	if (!ev->status) {
 		conn->handle = __le16_to_cpu(ev->handle);
 
-- 
2.34.1