--- /dev/null
+From ca1fd42e7dbfcb34890ffbf1f2f4b356776dab6f Mon Sep 17 00:00:00 2001
+From: Ruihan Li <lrh2000@pku.edu.cn>
+Date: Wed, 3 May 2023 21:39:34 +0800
+Subject: Bluetooth: Fix potential double free caused by hci_conn_unlink
+
+From: Ruihan Li <lrh2000@pku.edu.cn>
+
+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 <lrh2000@pku.edu.cn>
+Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
+Reported-by: syzbot+690b90b14f14f43f4688@syzkaller.appspotmail.com
+Reported-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
+Reported-by: syzbot+8bb72f86fc823817bc5d@syzkaller.appspotmail.com
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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)
--- /dev/null
+From a2ac591cb4d83e1f2d4b4adb3c14b2c79764650a Mon Sep 17 00:00:00 2001
+From: Ruihan Li <lrh2000@pku.edu.cn>
+Date: Wed, 3 May 2023 21:39:36 +0800
+Subject: Bluetooth: Fix UAF in hci_conn_hash_flush again
+
+From: Ruihan Li <lrh2000@pku.edu.cn>
+
+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 <lrh2000@pku.edu.cn>
+Co-developed-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
+Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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);
+ }
+ }
+
--- /dev/null
+From 2910431ab0e500dfc5df12299bb15eef0f30b43e Mon Sep 17 00:00:00 2001
+From: Ruihan Li <lrh2000@pku.edu.cn>
+Date: Wed, 3 May 2023 21:39:35 +0800
+Subject: Bluetooth: Refcnt drop must be placed last in hci_conn_unlink
+
+From: Ruihan Li <lrh2000@pku.edu.cn>
+
+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 <luiz.dentz@gmail.com>
+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 <lrh2000@pku.edu.cn>
+Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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;
+ }
projects / thirdparty / kernel / stable-queue.git / commitdiff
