From 28fde35e9d30fda89fe49d1efb7f98f545a5e93e Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 12 Jun 2023 10:31:18 +0200 Subject: [PATCH] 6.3-stable patches added patches: bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch --- ...ouble-free-caused-by-hci_conn_unlink.patch | 68 ++++++++++ ...fix-uaf-in-hci_conn_hash_flush-again.patch | 123 ++++++++++++++++++ ...st-be-placed-last-in-hci_conn_unlink.patch | 46 +++++++ queue-6.3/series | 3 + 4 files changed, 240 insertions(+) create mode 100644 queue-6.3/bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch create mode 100644 queue-6.3/bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch create mode 100644 queue-6.3/bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch diff --git a/queue-6.3/bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch b/queue-6.3/bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch new file mode 100644 index 00000000000..26b6b17d0c0 --- /dev/null +++ b/queue-6.3/bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch @@ -0,0 +1,68 @@ +From ca1fd42e7dbfcb34890ffbf1f2f4b356776dab6f Mon Sep 17 00:00:00 2001 +From: Ruihan Li +Date: Wed, 3 May 2023 21:39:34 +0800 +Subject: Bluetooth: Fix potential double free caused by hci_conn_unlink + +From: Ruihan Li + +commit ca1fd42e7dbfcb34890ffbf1f2f4b356776dab6f upstream. + +The hci_conn_unlink function is being called by hci_conn_del, which +means it should not call hci_conn_del with the input parameter conn +again. If it does, conn may have already been released when +hci_conn_unlink returns, leading to potential UAF and double-free +issues. + +This patch resolves the problem by modifying hci_conn_unlink to release +only conn's child links when necessary, but never release conn itself. + +Reported-by: syzbot+690b90b14f14f43f4688@syzkaller.appspotmail.com +Closes: https://lore.kernel.org/linux-bluetooth/000000000000484a8205faafe216@google.com/ +Fixes: 06149746e720 ("Bluetooth: hci_conn: Add support for linking multiple hcon") +Signed-off-by: Ruihan Li +Signed-off-by: Luiz Augusto von Dentz +Reported-by: syzbot+690b90b14f14f43f4688@syzkaller.appspotmail.com +Reported-by: Luiz Augusto von Dentz +Reported-by: syzbot+8bb72f86fc823817bc5d@syzkaller.appspotmail.com +Signed-off-by: Greg Kroah-Hartman +--- + net/bluetooth/hci_conn.c | 21 ++++++++++++--------- + 1 file changed, 12 insertions(+), 9 deletions(-) + +--- a/net/bluetooth/hci_conn.c ++++ b/net/bluetooth/hci_conn.c +@@ -1088,8 +1088,18 @@ static void hci_conn_unlink(struct hci_c + if (!conn->parent) { + struct hci_link *link, *t; + +- list_for_each_entry_safe(link, t, &conn->link_list, list) +- hci_conn_unlink(link->conn); ++ list_for_each_entry_safe(link, t, &conn->link_list, list) { ++ struct hci_conn *child = link->conn; ++ ++ hci_conn_unlink(child); ++ ++ /* 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. ++ */ ++ if (child->handle == HCI_CONN_HANDLE_UNSET) ++ hci_conn_del(child); ++ } + + return; + } +@@ -1105,13 +1115,6 @@ static void hci_conn_unlink(struct hci_c + + kfree(conn->link); + conn->link = NULL; +- +- /* 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. +- */ +- if (conn->handle == HCI_CONN_HANDLE_UNSET) +- hci_conn_del(conn); + } + + int hci_conn_del(struct hci_conn *conn) diff --git a/queue-6.3/bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch b/queue-6.3/bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch new file mode 100644 index 00000000000..813f35ac1ae --- /dev/null +++ b/queue-6.3/bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch @@ -0,0 +1,123 @@ +From a2ac591cb4d83e1f2d4b4adb3c14b2c79764650a Mon Sep 17 00:00:00 2001 +From: Ruihan Li +Date: Wed, 3 May 2023 21:39:36 +0800 +Subject: Bluetooth: Fix UAF in hci_conn_hash_flush again + +From: Ruihan Li + +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@syzkaller.appspotmail.com +Closes: https://lore.kernel.org/linux-bluetooth/000000000000aa920505f60d25ad@google.com/ +Fixes: 06149746e720 ("Bluetooth: hci_conn: Add support for linking multiple hcon") +Signed-off-by: Ruihan Li +Co-developed-by: Luiz Augusto von Dentz +Signed-off-by: Luiz Augusto von Dentz +Signed-off-by: Greg Kroah-Hartman +--- + 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); + } + } + diff --git a/queue-6.3/bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch b/queue-6.3/bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch new file mode 100644 index 00000000000..69fac53c7a1 --- /dev/null +++ b/queue-6.3/bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch @@ -0,0 +1,46 @@ +From 2910431ab0e500dfc5df12299bb15eef0f30b43e Mon Sep 17 00:00:00 2001 +From: Ruihan Li +Date: Wed, 3 May 2023 21:39:35 +0800 +Subject: Bluetooth: Refcnt drop must be placed last in hci_conn_unlink + +From: Ruihan Li + +commit 2910431ab0e500dfc5df12299bb15eef0f30b43e upstream. + +If hci_conn_put(conn->parent) reduces conn->parent's reference count to +zero, it can immediately deallocate conn->parent. At the same time, +conn->link->list has its head in conn->parent, causing use-after-free +problems in the latter list_del_rcu(&conn->link->list). + +This problem can be easily solved by reordering the two operations, +i.e., first performing the list removal with list_del_rcu and then +decreasing the refcnt with hci_conn_put. + +Reported-by: Luiz Augusto von Dentz +Closes: https://lore.kernel.org/linux-bluetooth/CABBYNZ+1kce8_RJrLNOXd_8=Mdpb=2bx4Nto-hFORk=qiOkoCg@mail.gmail.com/ +Fixes: 06149746e720 ("Bluetooth: hci_conn: Add support for linking multiple hcon") +Signed-off-by: Ruihan Li +Signed-off-by: Luiz Augusto von Dentz +Signed-off-by: Greg Kroah-Hartman +--- + net/bluetooth/hci_conn.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +--- a/net/bluetooth/hci_conn.c ++++ b/net/bluetooth/hci_conn.c +@@ -1107,12 +1107,12 @@ static void hci_conn_unlink(struct hci_c + if (!conn->link) + return; + +- hci_conn_put(conn->parent); +- conn->parent = NULL; +- + list_del_rcu(&conn->link->list); + synchronize_rcu(); + ++ hci_conn_put(conn->parent); ++ conn->parent = NULL; ++ + kfree(conn->link); + conn->link = NULL; + } diff --git a/queue-6.3/series b/queue-6.3/series index 2f503f933db..1cc4bbef4f9 100644 --- a/queue-6.3/series +++ b/queue-6.3/series @@ -150,3 +150,6 @@ ksmbd-fix-out-of-bound-read-in-deassemble_neg_contexts.patch ksmbd-fix-out-of-bound-read-in-parse_lease_state.patch ksmbd-fix-posix_acls-and-acls-dereferencing-possible-err_ptr.patch ksmbd-check-the-validation-of-pdu_size-in-ksmbd_conn_handler_loop.patch +bluetooth-fix-potential-double-free-caused-by-hci_conn_unlink.patch +bluetooth-refcnt-drop-must-be-placed-last-in-hci_conn_unlink.patch +bluetooth-fix-uaf-in-hci_conn_hash_flush-again.patch -- 2.47.2