From patchwork Fri Jun 23 17:18:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 695324 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2F7D0EB64DD for ; Fri, 23 Jun 2023 17:19:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232409AbjFWRTQ (ORCPT ); Fri, 23 Jun 2023 13:19:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47462 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232641AbjFWRS6 (ORCPT ); Fri, 23 Jun 2023 13:18:58 -0400 Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0748D26BD for ; Fri, 23 Jun 2023 10:18:50 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (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) (Authenticated sender: pav) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4QnkVJ0ZZMz49Q6x; Fri, 23 Jun 2023 20:18:48 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=83dDQcYbe5pUPhhpcZmbxd8AG+PU0UpwwamkIDyKTnc=; b=O1BRBGfQwA30r8jYsv/ICrk333+u0tmT5vJZUT90LXy9saBmNxa5PQi38n/x94ihgX4IEE 0L6fa/brW0rjIHm1AlikzezJ4OAQdqgZya9Hir8MJ3/PD0UeUF2NVsWLWlfUT3/ZUPs9SD A3YgDd3ZHKTITcUqEQgFGBk/2dTIaaD60iFmCvhzrsNU2JwK9+1hhwHcHuZ8Akn0JiDtap pmzVIlLc/ugIB+P5HsBm0UHE7A+L+az8XWRsdGoIPari5pqfgKm8dpYRT29x0PD9vmGcEv fqYSq0N5CtZtR7U8W0urUA9HMrzIZay5+w02mlFFJNM9idhi7hjbJXGa20ew2A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=83dDQcYbe5pUPhhpcZmbxd8AG+PU0UpwwamkIDyKTnc=; b=eA4220M6MRXLHtB3pGKTLnEnhjoTKdW4gf/SoR4s99ETkT3zfd4cN7eGnoJhpU3MbPqsyQ 8yUbGEqOJyTLSn1537jTDXcfx0Ro6zZdwWYh2b5TSL/1tYNPQJ7jVp4mmMR84PnlzOcfsL KWADrRC2UPT8mak+73wZvHcUtBCIdvSt9Aw1UY2TZv75qu4NkHHzZTZYnn297GCGD97ymb zph2nfALUJqbCtYX/mcyUzAzO+SH4igCsjSmWdwmc+uhV6usD5wQZBSU4OmNZNM1cdaGKT Ds6HmlDigoHXpsngZOGaMnGzHp+AAytcTyGDXFbgY7bdq17+bWMN3X4q5DQhYA== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1687540728; a=rsa-sha256; cv=none; b=u6QMHw2+eNq9m5YagDl2g2hQC2dHpkujvJGcz/Oxd++LHOVoG3r1fApGJfBKrx8/fYcAPL fh1c5jmFDzeGbJ41+5IVR4Pn24qpWz/W7pa/KFOUhgOqvOLn3cj8TO6Wg3qDuBCSCDXKi4 eqwDy0mTQlQv940ibTCmzhOBhufyMD+xuFONl1B0wxRmGiJzKFNr6sC9a8LloSpOw+uLuJ fgPyfCUBb4liABKkKG1AUdMG+JR6FOQW2xwN9hBE4BPw/OJEa/Eo1xnFeghOlw/zb+WZRJ BW91KPEyvBYXJyARXGGeNeG4AwDhBR3vlKFIdB6UD6HOsJ38KvP4XWfxMgIBEQ== From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 1/5] Bluetooth: hci_conn: add hci_conn_is_alive Date: Fri, 23 Jun 2023 20:18:38 +0300 Message-ID: <45455ee45ccb3313618a48c01be714e14d372257.1687525956.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org A delayed operation such as hci_sync on a given hci_conn needs to take hci_conn_get, so that the hci_conn doesn't get freed in the meantime. This does not guarantee the conn is still alive in a valid state, as it may be cleaned up in the meantime, so one needs to check if it is still in conn_hash to know if it's still alive. Simplify this alive check, using HCI_CONN_DELETED flag. This is also meaningful with RCU lock only, but with slightly different semantics. If hci_conn_is_alive(conn) returns true inside rcu_read_lock, conn was in conn_hash from the point of view of the current task when the flag was read. Then its deletion cannot complete before rcu_read_unlock. Signed-off-by: Pauli Virtanen --- Notes: This probably can be done with RCU primitives setting list.prev, but that's maybe more magical... include/net/bluetooth/hci_core.h | 18 ++++++++++++++++++ net/bluetooth/hci_conn.c | 10 +--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 05a9b3ab3f56..cab39bdd0592 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -978,6 +978,7 @@ enum { HCI_CONN_PER_ADV, HCI_CONN_BIG_CREATED, HCI_CONN_CREATE_CIS, + HCI_CONN_DELETED, }; static inline bool hci_conn_ssp_enabled(struct hci_conn *conn) @@ -997,6 +998,7 @@ static inline bool hci_conn_sc_enabled(struct hci_conn *conn) static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; + WARN_ON(test_bit(HCI_CONN_DELETED, &c->flags)); list_add_tail_rcu(&c->list, &h->list); switch (c->type) { case ACL_LINK: @@ -1023,6 +1025,10 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c) static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; + bool deleted; + + deleted = test_and_set_bit(HCI_CONN_DELETED, &c->flags); + WARN_ON(deleted); list_del_rcu(&c->list); synchronize_rcu(); @@ -1049,6 +1055,18 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c) } } +/* With hdev->lock: whether hci_conn is in conn_hash. + * With RCU: if true, the hci_conn is valid conn_hash iteration cursor and + * hci_conn_hash_del has not completed. (Note that if hci_conn was obtained in + * this critical section it is always valid, but this may return false!) + */ +static inline bool hci_conn_is_alive(struct hci_dev *hdev, struct hci_conn *c) +{ + RCU_LOCKDEP_WARN(lockdep_is_held(&hdev->lock) || rcu_read_lock_held(), + "suspicious locking"); + return !test_bit(HCI_CONN_DELETED, &c->flags); +} + static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type) { struct hci_conn_hash *h = &hdev->conn_hash; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 62a7ccfdfe63..d489a4829be7 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -183,21 +183,13 @@ static void le_scan_cleanup(struct work_struct *work) struct hci_conn *conn = container_of(work, struct hci_conn, le_scan_cleanup); struct hci_dev *hdev = conn->hdev; - struct hci_conn *c = NULL; BT_DBG("%s hcon %p", hdev->name, conn); hci_dev_lock(hdev); /* Check that the hci_conn is still around */ - rcu_read_lock(); - list_for_each_entry_rcu(c, &hdev->conn_hash.list, list) { - if (c == conn) - break; - } - rcu_read_unlock(); - - if (c == conn) { + if (hci_conn_is_alive(hdev, conn)) { hci_connect_le_scan_cleanup(conn, 0x00); hci_conn_cleanup(conn); } From patchwork Fri Jun 23 17:18:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 696282 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D553C001B0 for ; Fri, 23 Jun 2023 17:19:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232394AbjFWRTR (ORCPT ); Fri, 23 Jun 2023 13:19:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47482 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232653AbjFWRS6 (ORCPT ); Fri, 23 Jun 2023 13:18:58 -0400 Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0751E2705 for ; Fri, 23 Jun 2023 10:18:50 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (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) (Authenticated sender: pav) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4QnkVJ5gVqz49Q8L; Fri, 23 Jun 2023 20:18:48 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=w6pae5Alcoqq3vgYCiVKxWEBBEjBOhyN/nu/Ek04sVE=; b=CItbxaXbpDGihECOC2azjAkAcsVDzOWDw4n5Xjk6/PTHcimfKUH2K8Tf/+ZKbpv+IxGzWP UwXInAsRcT1uAT9sHXz/mkgKb+mdsBrzJ5y4LrvpkLwo7vnqw657UELzd7oYS+bk7hcd7J ttWTwY6naS7COoiGKm2DqbMZahtXlAaxNnbY2KAFmJPfH5HwyNLkOsGJ2KXXgGKNwtlhGm Dtkaj0bmagxKYdQhYt8EoNxqJmgjo8yL6BhMWl+vRkubr1EOacMXKw4xKG7qTvAIZ24H8S zzwl1l4RBtPf85BqQ71uXba5QQFJHpaqL9h+ZGquFbV2u0u9A2zKhK/sOManSA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=w6pae5Alcoqq3vgYCiVKxWEBBEjBOhyN/nu/Ek04sVE=; b=QYfdSWNRbtT170DdAS/OsGOkOW2x4CnPo10Y7LUOwHZHjUQxDy8PlNkZCZgBlsfJuX4aMY CPOYO9Tdbuta8msj57rW6lnzNEGPOi64LpVNIV+P5MXo6pUkZbQOVI62SU03EpbfcTXo+6 uMleiIrgZvGWv/e16cGqeXOy/+Nf+Gfl4BhTRfXQdWp7TMRw8YqOW1C6rPpmTWRvljzuUr inqCCa93pQ/WPUKZj6S/yiJ/sMp7oFDOzjjQXeAZyEhi8aSsU65RiP14YmXN1+5qCX7rsw I9TgkF636xydEpT0ZWhkkSxRaiPZ42BGKc2GBZ/JIXMlFYVEv+NdSu9aYkgxeg== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1687540728; a=rsa-sha256; cv=none; b=BQU7zlPK4s8O/rQybd42AEj6b6edcRHfwfKM7Lwyg4xlYpEtEOXu4zTLZ2eGiR0G0Wb36o JL7mT7HjHsjMU6cekqIxNnEvMW2lOZQlKRpnyX5da6cJoesklRyHFGs2OkLTqJyacuFdlp f7aX12mNuAzl/R+keoodBeIRGpnIxOmxqhIBNsVUuO3tGbH43JzXmz/H2U7FTfEUT1GhvW HZUlF06cr5tKWSJIQH4L3+/dnmotgZckP0EPg8FV9CQN32XGTCy1faPoZe0OEXw2GUNcP3 cgiGOr9v2brH2Dr5VqkoLpNnXK9sXPhf1PMtt0cjRQCfk1oyvxty04a7G5YzDQ== From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 2/5] Bluetooth: hci_sync: iterate conn_hash safely in hci_disconnect_all_sync Date: Fri, 23 Jun 2023 20:18:39 +0300 Message-ID: X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Any element can be removed from conn_hash when no rcu or hdev->lock, so list_for_each_entry_safe alone is not safe here. Add conn_hash iteration that uses RCU and takes hci_conn_get to keep cursors alive, to allow unlocked loop body safely. Since any item may then be deleted from conn_hash while locks are released, in rare cases (next item from cursor deleted) the iteration needs to be restarted. To process each item only once even if restarted, set HCI_CONN_CANCEL in hci_abort_conn_sync, similarly to what hci_abort_conn does. Log trace: ================================================================== BUG: KASAN: slab-use-after-free in hci_set_powered_sync (net/bluetooth/hci_sync.c:5345 net/bluetooth/hci_sync.c:5385 net/bluetooth/hci_sync.c:5397) bluetooth Read of size 8 at addr ffff88800a4d9000 by task kworker/u5:2/966 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 Workqueue: hci0 hci_cmd_sync_work [bluetooth] Call Trace: dump_stack_lvl (lib/dump_stack.c:108) print_report (mm/kasan/report.c:352 mm/kasan/report.c:462) ? __virt_addr_valid (./include/linux/mmzone.h:1901 ./include/linux/mmzone.h:1997 arch/x86/mm/physaddr.c:65) ? hci_set_powered_sync (net/bluetooth/hci_sync.c:5345 net/bluetooth/hci_sync.c:5385 net/bluetooth/hci_sync.c:5397) bluetooth kasan_report (mm/kasan/report.c:574) ? hci_set_powered_sync (net/bluetooth/hci_sync.c:5345 net/bluetooth/hci_sync.c:5385 net/bluetooth/hci_sync.c:5397) bluetooth hci_set_powered_sync (net/bluetooth/hci_sync.c:5345 net/bluetooth/hci_sync.c:5385 net/bluetooth/hci_sync.c:5397) bluetooth ? __pfx_hci_set_powered_sync (net/bluetooth/hci_sync.c:5393) bluetooth ? set_powered_sync (net/bluetooth/mgmt.c:1369) bluetooth ? __pfx_set_powered_sync (net/bluetooth/mgmt.c:1367) bluetooth hci_cmd_sync_work (net/bluetooth/hci_sync.c:306) bluetooth process_one_work (kernel/workqueue.c:2410) ? __pfx_process_one_work (kernel/workqueue.c:2300) ? __pfx_do_raw_spin_lock (kernel/locking/spinlock_debug.c:113) ? mark_held_locks (kernel/locking/lockdep.c:4240) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2553) ? __pfx_worker_thread (kernel/workqueue.c:2495) kthread (kernel/kthread.c:379) ? __pfx_kthread (kernel/kthread.c:332) ret_from_fork (arch/x86/entry/entry_64.S:314) Allocated by task 2366: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) __kasan_kmalloc (mm/kasan/common.c:374 mm/kasan/common.c:383) hci_conn_add (./include/linux/slab.h:559 ./include/linux/slab.h:680 net/bluetooth/hci_conn.c:1002) bluetooth hci_bind_cis (net/bluetooth/hci_conn.c:1908) bluetooth iso_connect_cis (net/bluetooth/iso.c:383) bluetooth iso_sock_connect (net/bluetooth/iso.c:890) bluetooth __sys_connect (./include/linux/file.h:44 net/socket.c:2021) __x64_sys_connect (net/socket.c:2027) do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) Freed by task 2366: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) kasan_save_free_info (mm/kasan/generic.c:523) __kasan_slab_free (mm/kasan/common.c:238 mm/kasan/common.c:200 mm/kasan/common.c:244) __kmem_cache_free (mm/slub.c:1807 mm/slub.c:3786 mm/slub.c:3799) device_release (drivers/base/core.c:2489) kobject_put (lib/kobject.c:683 lib/kobject.c:714 ./include/linux/kref.h:65 lib/kobject.c:731) __iso_sock_close (net/bluetooth/iso.c:665) bluetooth iso_sock_release (net/bluetooth/iso.c:686 net/bluetooth/iso.c:1473) bluetooth __sock_release (net/socket.c:654) sock_close (net/socket.c:1399) __fput (fs/file_table.c:322) task_work_run (kernel/task_work.c:180) exit_to_user_mode_prepare (./include/linux/resume_user_mode.h:49 kernel/entry/common.c:171 kernel/entry/common.c:204) syscall_exit_to_user_mode (kernel/entry/common.c:130 kernel/entry/common.c:299) do_syscall_64 (arch/x86/entry/common.c:87) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) ================================================================== Fixes: 182ee45da083 ("Bluetooth: hci_sync: Rework hci_suspend_notifier") Signed-off-by: Pauli Virtanen --- net/bluetooth/hci_sync.c | 140 +++++++++++++++++++++++++++++++++++---- 1 file changed, 127 insertions(+), 13 deletions(-) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index afb8e970e62c..46a156b44a8b 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5276,9 +5276,6 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev, if (test_bit(HCI_CONN_SCANNING, &conn->flags)) return 0; - if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) - return 0; - return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL, HCI_CMD_TIMEOUT); } @@ -5334,6 +5331,14 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) { int err; + /* No hdev->lock: but only accessing dst/type (immutable) and + * state/flags here, in worst case we just send some unnecessary + * HCI commands. + */ + + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) + return 0; + switch (conn->state) { case BT_CONNECTED: case BT_CONFIG: @@ -5342,10 +5347,12 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) err = hci_connect_cancel_sync(hdev, conn); /* Cleanup hci_conn object if it cannot be cancelled as it * likelly means the controller and host stack are out of sync. + * Watch out for deleted conn in calling conn_failed. */ if (err) { hci_dev_lock(hdev); - hci_conn_failed(conn, err); + if (hci_conn_is_alive(hdev, conn)) + hci_conn_failed(conn, err); hci_dev_unlock(hdev); } return err; @@ -5359,20 +5366,125 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) return 0; } -static int hci_disconnect_all_sync(struct hci_dev *hdev, u8 reason) +typedef bool (*hci_conn_iter_func_t)(struct hci_dev *hdev, + struct hci_conn *conn, + void *data); + +/* Iterate connections with unlocked loop body, allowing concurrent mutation, + * holding references to the cursors. If both the cursor and the next item are + * deleted while unlocked, this fails with -EBUSY, or optionally retries + * iteration from start. Note that hci_conn_cleanup may be running concurrently + * or have already completed for the conn, which you need to deal with. + */ +static int hci_conn_hash_list_unlocked(struct hci_dev *hdev, + bool retry, + hci_conn_iter_func_t func, + void *data) { - struct hci_conn *conn, *tmp; - int err; + struct list_head *head = &hdev->conn_hash.list; + struct hci_conn *pos, *prev, *prev_next; - list_for_each_entry_safe(conn, tmp, &hdev->conn_hash.list, list) { - err = hci_abort_conn_sync(hdev, conn, reason); - if (err) - return err; + if (!func) + return 0; + +again: + rcu_read_lock(); + + prev = NULL; + prev_next = NULL; + + pos = list_first_or_null_rcu(head, struct hci_conn, list); + if (pos) + hci_conn_get(pos); + + while (pos) { + struct list_head *ptr = &pos->list; + struct hci_conn *next; + + next = list_next_or_null_rcu(head, ptr, struct hci_conn, list); + if (next) + hci_conn_get(next); + + rcu_read_unlock(); + + /* Can't unref in RCU, so do it here */ + if (prev) { + hci_conn_put(prev); + prev = NULL; + } + + if (prev_next) { + hci_conn_put(prev_next); + prev_next = NULL; + } + + if (func(hdev, pos, data)) { + hci_conn_put(pos); + if (next) + hci_conn_put(next); + + return 0; + } + + rcu_read_lock(); + + if (next && !hci_conn_is_alive(hdev, next)) { + if (!hci_conn_is_alive(hdev, pos)) { + /* Both cursors deleted */ + rcu_read_unlock(); + hci_conn_put(pos); + hci_conn_put(next); + + if (retry) + goto again; + + return -EBUSY; + } + + /* Use the other cursor */ + prev_next = next; + next = list_next_or_null_rcu(head, ptr, + struct hci_conn, list); + if (next) + hci_conn_get(next); + } + + prev = pos; + pos = next; } + rcu_read_unlock(); + + if (prev) + hci_conn_put(prev); + if (prev_next) + hci_conn_put(prev_next); + return 0; } +struct disconnect_all_info { + u8 reason; + int err; +}; + +static bool disconnect_all_sync(struct hci_dev *hdev, struct hci_conn *conn, + void *data) +{ + struct disconnect_all_info *info = data; + + info->err = hci_abort_conn_sync(hdev, conn, info->reason); + return info->err; +} + +static int hci_disconnect_all_sync(struct hci_dev *hdev, u8 reason) +{ + struct disconnect_all_info info = {reason, 0}; + + hci_conn_hash_list_unlocked(hdev, true, disconnect_all_sync, &info); + return info.err; +} + /* This function perform power off HCI command sequence as follows: * * Clear Advertising @@ -6254,8 +6366,10 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) conn->conn_timeout, NULL); done: - if (err == -ETIMEDOUT) - hci_le_connect_cancel_sync(hdev, conn); + if (err == -ETIMEDOUT) { + if (!test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) + hci_le_connect_cancel_sync(hdev, conn); + } /* Re-enable advertising after the connection attempt is finished. */ hci_resume_advertising_sync(hdev); From patchwork Fri Jun 23 17:18:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 695323 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C34EEB64D7 for ; Fri, 23 Jun 2023 17:19:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232507AbjFWRTS (ORCPT ); Fri, 23 Jun 2023 13:19:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47478 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232648AbjFWRS6 (ORCPT ); Fri, 23 Jun 2023 13:18:58 -0400 Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AD592720 for ; Fri, 23 Jun 2023 10:18:51 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (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) (Authenticated sender: pav) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4QnkVJ6C0Yz49Q8P; Fri, 23 Jun 2023 20:18:48 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u8MhKiL0OH61fimV2NI4j0tPLHX8It4pkIfD4H/+7Ek=; b=ZwY8kp85r7SGMN+cpziRPacH927YP9A/hyjtmB4XfpyTTj14uOy1YYbj9Q9x9h7ie1Or1n zlsYyz15GtngCEWhGjKKhl41tcHpOx5VjRDHr+h3K3QxfUUtNPHYNBReedPpzq+N9CMmX7 V+VaplCq5bYe2+I9cSZErt5lEx2FOfjTmAtSQ4wdkA304VQam5V6gHdcluF9TrR88zQ9Uc PQEOnua+DIAvnAEaQHPUBUab1bsWyc3e2FPcW31rqO7GZNybUUy8uE++Pau9y2kOPJJvR7 389MSy3K2R8VGxB7BNWhHVNXy8CSiyBY1JPAQcJV1mY64iSB+ig1V+vRMg9AKQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540728; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=u8MhKiL0OH61fimV2NI4j0tPLHX8It4pkIfD4H/+7Ek=; b=BTPUwO4rL54hapt601+FJMR5z2ArNYxjLgpH/ArkkaI8v3iYiRbMpyKoN0Fa1fO30z/IcP yTmSR6nwhtUxvGCFa9A+Cg8Tu83MOqGmE3xwhkJKlP5S32uAr9em05ggInwDARL6ygtSTv 0W24kEa/WpSMyX/otKcNi2jeiY+8Z8Ki6GbPsq1XleCGVYP4YGuJ/BU403ABi7xKjJWjkg BLseCsYpSA6DmrL3b2g1i+8BfFNfYsWU5wu++9rLWNw+GSd9MBSihcOqpL+8V/8zwpJvlq 26TwWuXA1PFtoRq/cEKyj0HmvFprsuDZhbPvRiKSZs2wufinBguN4Jz70OSCFQ== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1687540728; a=rsa-sha256; cv=none; b=Fpqm1Re8jgT8eJmrqEGQeFSJQ4nsS/UBTWSlO5pAJUxO5XA8WSBLQdRhPJmaOasMeu1jBC a3NMUqvlSCVLAYI50RDCOfFAvT7rQ/I3npDXlt+PlJGibG+XCyxbXN+iXi6w7afRCgIByi ySSGForLxpZl4f+rSGHdwTVQZU75DgrubqMPeXrwdcxCTDO8JZYs13MFNCVFISMeivnFCm cf3fIi2VgoT9DMeWWRsl0b8IO3boCKY0pTh8ycn3vOR8O04hgg9fyizAyRiCwwf8dUNCHS IQpNdGzErBFbWgpDno7uSp3ij9n2aXVHvtNjPeH51c5BViZpyut/UCdAr1L/MA== From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 3/5] Bluetooth: hci_conn: hold ref while hci_connect_le_sync is pending Date: Fri, 23 Jun 2023 20:18:40 +0300 Message-ID: <42201414f52949665770da6229c9946c0d0dd5d8.1687525956.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org In hci_connect_le_sync, the hci_conn may be deleted before the hci_sync event runs. Hold a reference to the conn while the hci_sync command is queued, so we at least avoid the use-after-free. Add some checks to catch a race while the hci_sync command is queued, but this is still not quite right because the conn may be deleted during hci_le_create_conn_sync (but now we hold ref, so UAF less likely). The problem: ================================================================== BUG: KASAN: slab-use-after-free in hci_le_create_conn_sync (net/bluetooth/hci_sync.c:6167) bluetooth Read of size 1 at addr ffff88804c3ec03a by task kworker/u5:0/110 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 Workqueue: hci0 hci_cmd_sync_work [bluetooth] Call Trace: dump_stack_lvl (lib/dump_stack.c:108) print_report (mm/kasan/report.c:352 mm/kasan/report.c:462) ? __virt_addr_valid (./include/linux/mmzone.h:1901 ./include/linux/mmzone.h:1997 arch/x86/mm/physaddr.c:65) ? hci_le_create_conn_sync (net/bluetooth/hci_sync.c:6167) bluetooth kasan_report (mm/kasan/report.c:574) ? hci_le_create_conn_sync (net/bluetooth/hci_sync.c:6167) bluetooth hci_le_create_conn_sync (net/bluetooth/hci_sync.c:6167) bluetooth ? __pfx_hci_le_create_conn_sync (net/bluetooth/hci_sync.c:6160) bluetooth ? vsnprintf (lib/vsprintf.c:2890) ? vsnprintf (lib/vsprintf.c:2890) ? __dynamic_pr_debug (lib/dynamic_debug.c:858) ? __pfx___dynamic_pr_debug (lib/dynamic_debug.c:858) ? hci_cmd_sync_work (net/bluetooth/hci_sync.c:305) bluetooth ? __pfx_hci_connect_le_sync (net/bluetooth/hci_conn.c:1311) bluetooth hci_cmd_sync_work (net/bluetooth/hci_sync.c:306) bluetooth process_one_work (kernel/workqueue.c:2410) ? __pfx_process_one_work (kernel/workqueue.c:2300) ? __pfx_do_raw_spin_lock (kernel/locking/spinlock_debug.c:113) ? mark_held_locks (kernel/locking/lockdep.c:4240) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2553) ? __pfx_worker_thread (kernel/workqueue.c:2495) kthread (kernel/kthread.c:379) ? __pfx_kthread (kernel/kthread.c:332) ret_from_fork (arch/x86/entry/entry_64.S:314) Allocated by task 1321: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) __kasan_kmalloc (mm/kasan/common.c:374 mm/kasan/common.c:383) hci_conn_add (./include/linux/slab.h:559 ./include/linux/slab.h:680 net/bluetooth/hci_conn.c:1002) bluetooth hci_connect_le (net/bluetooth/hci_conn.c:1374) bluetooth process_adv_report.constprop.0 (net/bluetooth/hci_event.c:6172 net/bluetooth/hci_event.c:6293) bluetooth hci_le_ext_adv_report_evt (net/bluetooth/hci_event.c:6527) bluetooth hci_event_packet (net/bluetooth/hci_event.c:7515 net/bluetooth/hci_event.c:7570) bluetooth hci_rx_work (net/bluetooth/hci_core.c:4085) bluetooth process_one_work (kernel/workqueue.c:2410) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2553) kthread (kernel/kthread.c:379) ret_from_fork (arch/x86/entry/entry_64.S:314) Freed by task 110: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) kasan_save_free_info (mm/kasan/generic.c:523) __kasan_slab_free (mm/kasan/common.c:238 mm/kasan/common.c:200 mm/kasan/common.c:244) __kmem_cache_free (mm/slub.c:1807 mm/slub.c:3786 mm/slub.c:3799) device_release (drivers/base/core.c:2489) kobject_put (lib/kobject.c:683 lib/kobject.c:714 ./include/linux/kref.h:65 lib/kobject.c:731) hci_conn_hash_flush (net/bluetooth/hci_conn.c:2560) bluetooth hci_dev_close_sync (net/bluetooth/hci_sync.c:5021) bluetooth hci_dev_do_close (net/bluetooth/hci_core.c:556) bluetooth process_one_work (kernel/workqueue.c:2410) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2553) kthread (kernel/kthread.c:379) ret_from_fork (arch/x86/entry/entry_64.S:314) ================================================================== Signed-off-by: Pauli Virtanen --- net/bluetooth/hci_conn.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index d489a4829be7..d6fd00ba243f 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1280,6 +1280,9 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err) hci_dev_lock(hdev); + if (!hci_conn_is_alive(hdev, conn)) + goto done; + if (!err) { hci_connect_le_scan_cleanup(conn, 0x00); goto done; @@ -1295,6 +1298,7 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err) done: hci_dev_unlock(hdev); + hci_conn_put(conn); } static int hci_connect_le_sync(struct hci_dev *hdev, void *data) @@ -1303,6 +1307,14 @@ static int hci_connect_le_sync(struct hci_dev *hdev, void *data) bt_dev_dbg(hdev, "conn %p", conn); + /* TODO: fix conn race conditions in hci_sync, this is not enough */ + hci_dev_lock(hdev); + if (!hci_conn_is_alive(hdev, conn)) { + hci_dev_unlock(hdev); + return -ECANCELED; + } + hci_dev_unlock(hdev); + return hci_le_create_conn_sync(hdev, conn); } @@ -1375,9 +1387,11 @@ struct hci_conn *hci_connect_le(struct hci_dev *hdev, bdaddr_t *dst, conn->state = BT_CONNECT; clear_bit(HCI_CONN_SCANNING, &conn->flags); + hci_conn_get(conn); err = hci_cmd_sync_queue(hdev, hci_connect_le_sync, conn, create_le_conn_complete); if (err) { + hci_conn_put(conn); hci_conn_del(conn); return ERR_PTR(err); } From patchwork Fri Jun 23 17:18:41 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 696281 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C852C001B0 for ; Fri, 23 Jun 2023 17:19:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232444AbjFWRTT (ORCPT ); Fri, 23 Jun 2023 13:19:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47474 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232645AbjFWRS6 (ORCPT ); Fri, 23 Jun 2023 13:18:58 -0400 Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [185.185.170.37]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 079732706 for ; Fri, 23 Jun 2023 10:18:50 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (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) (Authenticated sender: pav) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4QnkVJ6tNqz49QDV; Fri, 23 Jun 2023 20:18:48 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540729; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QE7Wmh8DHA08SHAlsY8fqP2kmIzXClUV+WNikrrqgyk=; b=tmBRrTobEEIeTo4161+MupKTVCLHyskrePBnvYS7HyK+of8ufakcmxCEIv7rmN0B510K1H Tw3TzxXP74XKAkOlnoxNhDxb5pNNEy7qIJwoGd+a9tI35GCZIWgTEmGq8mez4cJgf84fQi vhmxpoqVfE+rPXQ9eqid1kmo/s4hYV+hktZGUPZ2/r4Nd6IbMeXVfLYvW8QjJm5iIyuxgp NQ7DjCDsnJeC8FKoJKcxhJ8iz0Dx8WH7ZNXCWgsFAJ9dAgBe+LEw9hCrlba0OM1M9CS0VL p1MRJ74Y9T89PDGX1ChCERedxUnxoyS0xOOQDRnpu9q5g10OoEgZ+oKMAW+bPg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540729; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QE7Wmh8DHA08SHAlsY8fqP2kmIzXClUV+WNikrrqgyk=; b=l8XUDU3/DZezpJZNGfADiPH/Sn8QUbIntNZycQ4ljE1Ox1w/RI+39bpFMo8R7A5sAlp4cg +tt8spX/mb51RGa39QKrXEAYqPE9b4QaFod7Cb+nMRyEeG1grdJ1HFrvv/xk7/39TQLiMQ DT0+P2D/9YWsU9vKC9xX2/Bw5Yf0+mx7wMqDCZ6IXzZsAY5jzyCNApKvjyK0kL4TQlulnP CxCblIYoDhDfndv2iIYFLcO8qU9yUqzXbUtFzxImN/swNMny9chKy+XcaXxGC1KwwXG0aW IgMH9dM3zcq0PwKQdPSPD+AFM+F+oKIXKYTX7dTkSO72kVh4FZS7KXIGWVnUIA== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1687540729; a=rsa-sha256; cv=none; b=k457qJ+TtKKYDEoi0cYM9YWqVUsbroQDqddnJnDycbEXJq135v6YzrcyaoAC4smtaLcIsU jmPpgkZoTCoesQI97vvsng514Na6U8wgDVeIa4Qdb1la55oRCX/duHjJk8mYE2AuuH1Pvx FoyJnRUy8lOPyL0a12CaqfwOBi7hNMNNk3+Dyb4f0y+H8AplI72OPhJYvxHvReZso/QKzP mzMSPfOtvh5YuMAo8kFV447ISGSyLEQE720u3oUF0/uXPfji10XHF5L9zeYsguOZiN0uIv XRuhQmlQpyQf0KfnO8RH4ez7Dag2F/0C0/eqycmoPStoKT4K5Azbhhbr1renRw== From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 4/5] Bluetooth: ISO: fix use-after-free in __iso_sock_close Date: Fri, 23 Jun 2023 20:18:41 +0300 Message-ID: <9fc59119ddbed06455cc8056ddf3c9968fd3ad11.1687525956.git.pav@iki.fi> X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org __iso_sock_close calls hci_conn_del without hci_dev_lock, which is invalid and results to use-after-free under suitable conditions. But the lock cannot be taken here because of lock ordering hci_dev_lock > lock_sock > iso_conn_lock. Instead, take hci_conn_get temporarily, and delete the connection after lock_sock is released later on, if it is still alive. Also do iso_conn_del, so that we don't leak the iso_conn. Log: ================================================================== hci_chan_list_flush:2769: hcon ffff8880356b9000 hci_le_remove_cig:952: hci0: handle 0x00 hci_dev_put:1487: hci0 orig refcnt 9 iso_chan_del:156: sk ffff888010918800, conn ffff888004daee00, err 104 iso_chan_del:156: sk ffff888010918800, conn 0000000000000000, err 103 BUG: KASAN: slab-use-after-free in iso_conn_del (net/bluetooth/iso.c:211) bluetooth Write of size 8 at addr ffff8880356b99f0 by task kworker/u5:1/1073 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014 Workqueue: hci0 hci_cmd_sync_work [bluetooth] Call Trace: dump_stack_lvl (lib/dump_stack.c:108) print_report (mm/kasan/report.c:352 mm/kasan/report.c:462) ? __virt_addr_valid (./include/linux/mmzone.h:1901 ./include/linux/mmzone.h:1997 arch/x86/mm/physaddr.c:65) ? iso_conn_del (net/bluetooth/iso.c:211) bluetooth kasan_report (mm/kasan/report.c:574) ? iso_conn_del (net/bluetooth/iso.c:211) bluetooth iso_conn_del (net/bluetooth/iso.c:211) bluetooth hci_conn_hash_flush (./include/net/bluetooth/hci_core.h:1872 net/bluetooth/hci_conn.c:2576) bluetooth hci_dev_close_sync (net/bluetooth/hci_sync.c:5043) bluetooth ? __pfx_hci_dev_close_sync (net/bluetooth/hci_sync.c:4974) bluetooth hci_set_powered_sync (net/bluetooth/hci_sync.c:5433 net/bluetooth/hci_sync.c:5441) bluetooth ? __pfx_hci_set_powered_sync (net/bluetooth/hci_sync.c:5437) bluetooth ? set_powered_sync (net/bluetooth/mgmt.c:1369) bluetooth ? __pfx_set_powered_sync (net/bluetooth/mgmt.c:1367) bluetooth hci_cmd_sync_work (net/bluetooth/hci_sync.c:306) bluetooth process_one_work (kernel/workqueue.c:2410) ? __pfx_process_one_work (kernel/workqueue.c:2300) ? __pfx_do_raw_spin_lock (kernel/locking/spinlock_debug.c:113) ? mark_held_locks (kernel/locking/lockdep.c:4240) worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2553) ? __pfx_worker_thread (kernel/workqueue.c:2495) kthread (kernel/kthread.c:379) ? __pfx_kthread (kernel/kthread.c:332) ret_from_fork (arch/x86/entry/entry_64.S:314) Allocated by task 2372: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) __kasan_kmalloc (mm/kasan/common.c:374 mm/kasan/common.c:383) hci_conn_add (./include/linux/slab.h:559 ./include/linux/slab.h:680 net/bluetooth/hci_conn.c:1002) bluetooth hci_bind_cis (net/bluetooth/hci_conn.c:1920) bluetooth iso_connect_cis (net/bluetooth/iso.c:383) bluetooth iso_sock_connect (net/bluetooth/iso.c:890) bluetooth __sys_connect (./include/linux/file.h:44 net/socket.c:2021) __x64_sys_connect (net/socket.c:2027) do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) Freed by task 2372: kasan_save_stack (mm/kasan/common.c:46) kasan_set_track (mm/kasan/common.c:52) kasan_save_free_info (mm/kasan/generic.c:523) __kasan_slab_free (mm/kasan/common.c:238 mm/kasan/common.c:200 mm/kasan/common.c:244) __kmem_cache_free (mm/slub.c:1807 mm/slub.c:3786 mm/slub.c:3799) device_release (drivers/base/core.c:2489) kobject_put (lib/kobject.c:683 lib/kobject.c:714 ./include/linux/kref.h:65 lib/kobject.c:731) __iso_sock_close (net/bluetooth/iso.c:665) bluetooth iso_sock_release (net/bluetooth/iso.c:686 net/bluetooth/iso.c:1473) bluetooth __sock_release (net/socket.c:654) sock_close (net/socket.c:1399) __fput (fs/file_table.c:322) task_work_run (kernel/task_work.c:180) exit_to_user_mode_prepare (./include/linux/resume_user_mode.h:49 kernel/entry/common.c:171 kernel/entry/common.c:204) syscall_exit_to_user_mode (kernel/entry/common.c:130 kernel/entry/common.c:299) do_syscall_64 (arch/x86/entry/common.c:87) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120) The buggy address belongs to the object at ffff8880356b9000 which belongs to the cache kmalloc-4k of size 4096 The buggy address is located 2544 bytes inside of freed 4096-byte region [ffff8880356b9000, ffff8880356ba000) ================================================================== Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type") Signed-off-by: Pauli Virtanen --- net/bluetooth/iso.c | 50 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index 84d238d0639a..ea0209fb9872 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -626,7 +626,38 @@ static void iso_conn_defer_reject(struct hci_conn *conn) hci_send_cmd(conn->hdev, HCI_OP_LE_REJECT_CIS, sizeof(cp), &cp); } -static void __iso_sock_close(struct sock *sk) +static void iso_conn_del_hci_conn(struct iso_conn *conn, struct hci_conn **del) +{ + /* Lock ordering forbids taking hdev->lock, postpone hci_conn_del */ + iso_conn_lock(conn); + if (conn->hcon) { + hci_conn_get(conn->hcon); + hci_dev_hold(conn->hcon->hdev); + *del = conn->hcon; + conn->hcon = NULL; + } + iso_conn_unlock(conn); +} + +static void iso_conn_del_hci_conn_finish(struct hci_conn *hcon) +{ + struct hci_dev *hdev; + + if (!hcon) + return; + + hdev = hcon->hdev; + hci_dev_lock(hdev); + if (hci_conn_is_alive(hdev, hcon)) { + iso_conn_del(hcon, ECONNRESET); + hci_conn_del(hcon); + } + hci_dev_unlock(hdev); + hci_dev_put(hdev); + hci_conn_put(hcon); +} + +static void __iso_sock_close(struct sock *sk, struct hci_conn **del_conn) { BT_DBG("sk %p state %d socket %p", sk, sk->sk_state, sk->sk_socket); @@ -659,11 +690,8 @@ static void __iso_sock_close(struct sock *sk) * needs to be removed so just call hci_conn_del so the cleanup * callback do what is needed. */ - if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags) && - iso_pi(sk)->conn->hcon) { - hci_conn_del(iso_pi(sk)->conn->hcon); - iso_pi(sk)->conn->hcon = NULL; - } + if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) + iso_conn_del_hci_conn(iso_pi(sk)->conn, del_conn); iso_chan_del(sk, ECONNRESET); break; @@ -680,11 +708,14 @@ static void __iso_sock_close(struct sock *sk) /* Must be called on unlocked socket. */ static void iso_sock_close(struct sock *sk) { + struct hci_conn *del_conn = NULL; + iso_sock_clear_timer(sk); lock_sock(sk); - __iso_sock_close(sk); + __iso_sock_close(sk, &del_conn); release_sock(sk); iso_sock_kill(sk); + iso_conn_del_hci_conn_finish(del_conn); } static void iso_sock_init(struct sock *sk, struct sock *parent) @@ -1418,6 +1449,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, static int iso_sock_shutdown(struct socket *sock, int how) { struct sock *sk = sock->sk; + struct hci_conn *del_conn = NULL; int err = 0; BT_DBG("sock %p, sk %p, how %d", sock, sk, how); @@ -1447,7 +1479,7 @@ static int iso_sock_shutdown(struct socket *sock, int how) } iso_sock_clear_timer(sk); - __iso_sock_close(sk); + __iso_sock_close(sk, &del_conn); if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime && !(current->flags & PF_EXITING)) @@ -1457,6 +1489,8 @@ static int iso_sock_shutdown(struct socket *sock, int how) release_sock(sk); sock_put(sk); + iso_conn_del_hci_conn_finish(del_conn); + return err; } From patchwork Fri Jun 23 17:18:42 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pauli Virtanen X-Patchwork-Id: 695322 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8891EEB64DD for ; Fri, 23 Jun 2023 17:19:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232414AbjFWRTU (ORCPT ); Fri, 23 Jun 2023 13:19:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47514 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232620AbjFWRTA (ORCPT ); Fri, 23 Jun 2023 13:19:00 -0400 Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 221A41997 for ; Fri, 23 Jun 2023 10:18:59 -0700 (PDT) Received: from monolith.lan (91-152-120-101.elisa-laajakaista.fi [91.152.120.101]) (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) (Authenticated sender: pav) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4QnkVK0RyXz49QK5; Fri, 23 Jun 2023 20:18:49 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540729; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zaCe5l9sNZdr/d/DwckHXoXDhHv0aArdv7bPevQmsZQ=; b=PP8hMMJatUJu323qjIPOT3/Zc/qr6LDLVBDTFpF0ksAv55cLAAJ/5ICZImKD3cGGvft9M8 XLeNmVAiBerU0mntFIcSYQttp5MvGnaTuPcwf6FLLBF2bqIqIuqgIf3y77VBfuV4k+4OGz EmSbSJEt5eLQltyBxf1eVY4Gal3D2OMkK42dv1S6yEL3GCAJFwq+NvLLfYvXjIgvLyliOi qej7jiz/cTRmTJBCXcLPAJOHQDH/IIoPiVJjC9ttw0DanTlkqHIUkh/bSJ+zI3QY/5zgoY maMEhXSqSfwSDw53Kk4HfshuNQ9P4FhFYgsgOQTKmQrBc1pdtCcfhTRnWOMGqQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1687540729; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=zaCe5l9sNZdr/d/DwckHXoXDhHv0aArdv7bPevQmsZQ=; b=d5TgFgcULHDmAE+I6GnlgXyGTodsNpb8fG2V3f+Yahg2ZQjzwOHu1+I5DRXpy7uDVqxott DyV9V7h0Q5qay20HR1AD5qbqTjToGD2y09o2osXcXBHACkfG1YjwvE0lyGOf5CqxRy6XYP bsWG1Ah/Oj4jW0sPA4UVuCDAUzgRfEfb908/97cDL8Go8w+QuBu5Ygfkmox3soNCh+h1ld f8W8HaZOIsAEs3q6sV0299ZCr2mMkFRgXsXjUAE3Dbe5zuJuJnnWQxpQ58li30gh2c1Tpo rcP5RJ4mJTEJbo3olwvYMkHL5t5cfyfDNwcL5QaYruRzRCliOGynLQjbzbgXLA== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=pav smtp.mailfrom=pav@iki.fi ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1687540729; a=rsa-sha256; cv=none; b=VYZCLYn54KUx8tWUqH+PK3x1eJkj/rstqeWVeGe4a/lSZNAqnUesKccv6zH2sSyEyUf/gv HYaKUDpZ1ONtOziHf/lZ065i4bMxj8x9gmSE/CK1u2QLSN5P9W/hTJyM52hhyd92vUfttJ eATTsKrcWN6L2l8CES4NIDrST4mTJlo4ImAlvAi+94LgD9kIfmFtRK6D2o35Mff8JJSICP Do0LPPr3hL+YgV7K30/3mcNt95bkIeZhUjHjH6SaM40CgCAJKiO2CvtqBaNizyoURiHVjF t9VSJT+9UH1CQsXaDwKnNvaKOaIjfpRiNuJhjJlxdogiT+i0uL7opq9BnBMNyg== From: Pauli Virtanen To: linux-bluetooth@vger.kernel.org Cc: Pauli Virtanen Subject: [PATCH RFC 5/5] Bluetooth: ISO: fix locking in iso_conn_ready Date: Fri, 23 Jun 2023 20:18:42 +0300 Message-ID: X-Mailer: git-send-email 2.41.0 In-Reply-To: References: MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Getting conn->sk must sock_hold, otherwise the socket may be freed concurrently. Access to conn->hcon is safe when holding hdev->lock. Fix the locking in iso_conn_ready to obey this. Signed-off-by: Pauli Virtanen --- net/bluetooth/iso.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index ea0209fb9872..c2045adbd7b6 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -179,6 +179,7 @@ static void iso_chan_del(struct sock *sk, int err) sock_set_flag(sk, SOCK_ZAPPED); } +/* Must hold hdev->lock */ static void iso_conn_del(struct hci_conn *hcon, int err) { struct iso_conn *conn = hcon->iso_data; @@ -1547,19 +1548,23 @@ static bool iso_match_big(struct sock *sk, void *data) static void iso_conn_ready(struct iso_conn *conn) { struct sock *parent; - struct sock *sk = conn->sk; + struct sock *sk; struct hci_ev_le_big_sync_estabilished *ev; struct hci_conn *hcon; BT_DBG("conn %p", conn); - if (sk) { - iso_sock_ready(conn->sk); - } else { - hcon = conn->hcon; - if (!hcon) - return; + iso_conn_lock(conn); + hcon = conn->hcon; + sk = conn->sk; + if (sk) + sock_hold(sk); + iso_conn_unlock(conn); + if (sk) { + iso_sock_ready(sk); + sock_put(sk); + } else if (hcon) { ev = hci_recv_event_data(hcon->hdev, HCI_EVT_LE_BIG_SYNC_ESTABILISHED); if (ev)