This is a note to let you know that I've just added the patch titled
Bluetooth: Fix UAF in hci_conn_hash_flush again
to the 6.3-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch
and it can be found in the queue-6.3 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From a2ac591cb4d83e1f2d4b4adb3c14b2c79764650a Mon Sep 17 00:00:00 2001
From: Ruihan Li <lrh2000@xxxxxxxxxx>
Date: Wed, 3 May 2023 21:39:36 +0800
Subject: Bluetooth: Fix UAF in hci_conn_hash_flush again
From: Ruihan Li <lrh2000@xxxxxxxxxx>
commit a2ac591cb4d83e1f2d4b4adb3c14b2c79764650a upstream.
Commit 06149746e720 ("Bluetooth: hci_conn: Add support for linking
multiple hcon") reintroduced a previously fixed bug [1] ("KASAN:
slab-use-after-free Read in hci_conn_hash_flush"). This bug was
originally fixed by commit 5dc7d23e167e ("Bluetooth: hci_conn: Fix
possible UAF").
The hci_conn_unlink function was added to avoid invalidating the link
traversal caused by successive hci_conn_del operations releasing extra
connections. However, currently hci_conn_unlink itself also releases
extra connections, resulted in the reintroduced bug.
This patch follows a more robust solution for cleaning up all
connections, by repeatedly removing the first connection until there are
none left. This approach does not rely on the inner workings of
hci_conn_del and ensures proper cleanup of all connections.
Meanwhile, we need to make sure that hci_conn_del never fails. Indeed it
doesn't, as it now always returns zero. To make this a bit clearer, this
patch also changes its return type to void.
Reported-by: syzbot+8bb72f86fc823817bc5d@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://lore.kernel.org/linux-bluetooth/000000000000aa920505f60d25ad@xxxxxxxxxx/
Fixes: 06149746e720 ("Bluetooth: hci_conn: Add support for linking multiple hcon")
Signed-off-by: Ruihan Li <lrh2000@xxxxxxxxxx>
Co-developed-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_conn.c | 33 ++++++++++++++++++++++-----------
2 files changed, 23 insertions(+), 12 deletions(-)
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1324,7 +1324,7 @@ int hci_le_create_cis(struct hci_conn *c
struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
u8 role);
-int hci_conn_del(struct hci_conn *conn);
+void hci_conn_del(struct hci_conn *conn);
void hci_conn_hash_flush(struct hci_dev *hdev);
void hci_conn_check_pending(struct hci_dev *hdev);
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1093,6 +1093,14 @@ static void hci_conn_unlink(struct hci_c
hci_conn_unlink(child);
+ /* If hdev is down it means
+ * hci_dev_close_sync/hci_conn_hash_flush is in progress
+ * and links don't need to be cleanup as all connections
+ * would be cleanup.
+ */
+ if (!test_bit(HCI_UP, &hdev->flags))
+ continue;
+
/* Due to race, SCO connection might be not established
* yet at this point. Delete it now, otherwise it is
* possible for it to be stuck and can't be deleted.
@@ -1117,7 +1125,7 @@ static void hci_conn_unlink(struct hci_c
conn->link = NULL;
}
-int hci_conn_del(struct hci_conn *conn)
+void hci_conn_del(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
@@ -1168,8 +1176,6 @@ int hci_conn_del(struct hci_conn *conn)
* rest of hci_conn_del.
*/
hci_conn_cleanup(conn);
-
- return 0;
}
struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)
@@ -2526,22 +2532,27 @@ timer:
/* Drop all connection on the device */
void hci_conn_hash_flush(struct hci_dev *hdev)
{
- struct hci_conn_hash *h = &hdev->conn_hash;
- struct hci_conn *c, *n;
+ struct list_head *head = &hdev->conn_hash.list;
+ struct hci_conn *conn;
BT_DBG("hdev %s", hdev->name);
- list_for_each_entry_safe(c, n, &h->list, list) {
- c->state = BT_CLOSED;
-
- hci_disconn_cfm(c, HCI_ERROR_LOCAL_HOST_TERM);
+ /* We should not traverse the list here, because hci_conn_del
+ * can remove extra links, which may cause the list traversal
+ * to hit items that have already been released.
+ */
+ while ((conn = list_first_entry_or_null(head,
+ struct hci_conn,
+ list)) != NULL) {
+ conn->state = BT_CLOSED;
+ hci_disconn_cfm(conn, HCI_ERROR_LOCAL_HOST_TERM);
/* Unlink before deleting otherwise it is possible that
* hci_conn_del removes the link which may cause the list to
* contain items already freed.
*/
- hci_conn_unlink(c);
- hci_conn_del(c);
+ hci_conn_unlink(conn);
+ hci_conn_del(conn);
}
}
Patches currently in stable-queue which might be from lrh2000@xxxxxxxxxx are
queue-6.3/bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch
queue-6.3/bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch
queue-6.3/usb-usbfs-enforce-page-requirements-for-mmap.patch
queue-6.3/mm-page_table_check-make-it-dependent-on-exclusive_system_ram.patch
queue-6.3/bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch
queue-6.3/usb-usbfs-use-consistent-mmap-functions.patch
queue-6.3/mm-page_table_check-ensure-user-pages-are-not-slab-pages.patch
