| Message ID | 20230403224412.2021594-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 14a833f37a7495d5d16bb411d55d0c688b451550 |
| Headers | show |
| Series | [v2] Bluetooth: hci_conn: Fix possible UAF | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | success | TestRunner PASS |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
Hi Archie, On Mon, Apr 3, 2023 at 3:44 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This fixes the following trace: > > ================================================================== > BUG: KASAN: slab-use-after-free in hci_conn_del+0xba/0x3a0 > Write of size 8 at addr ffff88800208e9c8 by task iso-tester/31 > > CPU: 0 PID: 31 Comm: iso-tester Not tainted 6.3.0-rc2-g991aa4a69a47 > #4716 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.1-2.fc36 > 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x1d/0x70 > print_report+0xce/0x610 > ? __virt_addr_valid+0xd4/0x150 > ? hci_conn_del+0xba/0x3a0 > kasan_report+0xdd/0x110 > ? hci_conn_del+0xba/0x3a0 > hci_conn_del+0xba/0x3a0 > hci_conn_hash_flush+0xf2/0x120 > hci_dev_close_sync+0x388/0x920 > hci_unregister_dev+0x122/0x260 > vhci_release+0x4f/0x90 > __fput+0x102/0x430 > task_work_run+0xf1/0x160 > ? __pfx_task_work_run+0x10/0x10 > ? mark_held_locks+0x24/0x90 > exit_to_user_mode_prepare+0x170/0x180 > syscall_exit_to_user_mode+0x19/0x50 > do_syscall_64+0x4e/0x90 > entry_SYSCALL_64_after_hwframe+0x70/0xda > > Fixes: 0f00cd322d22 ("Bluetooth: Free potentially unfreed SCO connection") > Link: https://syzkaller.appspot.com/bug?extid=8bb72f86fc823817bc5d > Cc: <stable@vger.kernel.org> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > net/bluetooth/hci_conn.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c > index fe8d54f8f04f..5672b4924572 100644 > --- a/net/bluetooth/hci_conn.c > +++ b/net/bluetooth/hci_conn.c > @@ -1069,6 +1069,17 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, > return conn; > } > > +static bool hci_conn_unlink(struct hci_conn *conn) > +{ > + if (!conn->link) > + return false; > + > + conn->link->link = NULL; > + conn->link = NULL; > + > + return true; > +} > + > int hci_conn_del(struct hci_conn *conn) > { > struct hci_dev *hdev = conn->hdev; > @@ -1080,15 +1091,16 @@ int hci_conn_del(struct hci_conn *conn) > cancel_delayed_work_sync(&conn->idle_work); > > if (conn->type == ACL_LINK) { > - struct hci_conn *sco = conn->link; > - if (sco) { > - sco->link = NULL; > + struct hci_conn *link = conn->link; > + > + if (link) { > + hci_conn_unlink(conn); > /* 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 (sco->handle == HCI_CONN_HANDLE_UNSET) > - hci_conn_del(sco); > + if (link->handle == HCI_CONN_HANDLE_UNSET) > + hci_conn_del(link); > } > > /* Unacked frames */ > @@ -1104,7 +1116,7 @@ int hci_conn_del(struct hci_conn *conn) > struct hci_conn *acl = conn->link; > > if (acl) { > - acl->link = NULL; > + hci_conn_unlink(conn); > hci_conn_drop(acl); > } > > @@ -2444,6 +2456,12 @@ void hci_conn_hash_flush(struct hci_dev *hdev) > c->state = BT_CLOSED; > > hci_disconn_cfm(c, 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); > } > } > -- > 2.39.2 Can you give it a try if this doesn't break the existing problem you were trying to fix, anyway it is quite easy to reproduce with the likes of iso-tester and syzbot also seems to have bumped into it as well.
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=736584 ---Test result--- Test Summary: CheckPatch PASS 0.72 seconds GitLint PASS 0.32 seconds SubjectPrefix PASS 0.10 seconds BuildKernel PASS 38.44 seconds CheckAllWarning PASS 41.94 seconds CheckSparse PASS 47.49 seconds CheckSmatch PASS 128.77 seconds BuildKernel32 PASS 37.21 seconds TestRunnerSetup PASS 529.38 seconds TestRunner_l2cap-tester PASS 18.42 seconds TestRunner_iso-tester PASS 18.87 seconds TestRunner_bnep-tester PASS 6.25 seconds TestRunner_mgmt-tester PASS 123.34 seconds TestRunner_rfcomm-tester PASS 9.76 seconds TestRunner_sco-tester PASS 8.91 seconds TestRunner_ioctl-tester PASS 10.46 seconds TestRunner_mesh-tester PASS 7.89 seconds TestRunner_smp-tester PASS 8.89 seconds TestRunner_userchan-tester PASS 6.52 seconds IncrementalBuild PASS 34.74 seconds --- Regards, Linux Bluetooth
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Mon, 3 Apr 2023 15:44:12 -0700 you wrote: > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > > This fixes the following trace: > > ================================================================== > BUG: KASAN: slab-use-after-free in hci_conn_del+0xba/0x3a0 > Write of size 8 at addr ffff88800208e9c8 by task iso-tester/31 > > [...] Here is the summary with links: - [v2] Bluetooth: hci_conn: Fix possible UAF https://git.kernel.org/bluetooth/bluetooth-next/c/14a833f37a74 You are awesome, thank you!
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index fe8d54f8f04f..5672b4924572 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1069,6 +1069,17 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, return conn; } +static bool hci_conn_unlink(struct hci_conn *conn) +{ + if (!conn->link) + return false; + + conn->link->link = NULL; + conn->link = NULL; + + return true; +} + int hci_conn_del(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; @@ -1080,15 +1091,16 @@ int hci_conn_del(struct hci_conn *conn) cancel_delayed_work_sync(&conn->idle_work); if (conn->type == ACL_LINK) { - struct hci_conn *sco = conn->link; - if (sco) { - sco->link = NULL; + struct hci_conn *link = conn->link; + + if (link) { + hci_conn_unlink(conn); /* 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 (sco->handle == HCI_CONN_HANDLE_UNSET) - hci_conn_del(sco); + if (link->handle == HCI_CONN_HANDLE_UNSET) + hci_conn_del(link); } /* Unacked frames */ @@ -1104,7 +1116,7 @@ int hci_conn_del(struct hci_conn *conn) struct hci_conn *acl = conn->link; if (acl) { - acl->link = NULL; + hci_conn_unlink(conn); hci_conn_drop(acl); } @@ -2444,6 +2456,12 @@ void hci_conn_hash_flush(struct hci_dev *hdev) c->state = BT_CLOSED; hci_disconn_cfm(c, 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); } }