2023-10-18 10:33:00

by Bastien Nocera

[permalink] [raw]
Subject: [PATCH v3] Bluetooth: Fix double free in hci_conn_cleanup

From: ZhengHan Wang <[email protected]>

syzbot reports a slab use-after-free in hci_conn_hash_flush [1].
After releasing an object using hci_conn_del_sysfs in the
hci_conn_cleanup function, releasing the same object again
using the hci_dev_put and hci_conn_put functions causes a double free.
Here's a simplified flow:

hci_conn_del_sysfs:
hci_dev_put
put_device
kobject_put
kref_put
kobject_release
kobject_cleanup
kfree_const
kfree(name)

hci_dev_put:
...
kfree(name)

hci_conn_put:
put_device
...
kfree(name)

This patch drop the hci_dev_put and hci_conn_put function
call in hci_conn_cleanup function, because the object is
freed in hci_conn_del_sysfs function.

This patch also fixes the refcounting in hci_conn_add_sysfs() and
hci_conn_del_sysfs() to take into account device_add() failures.

This fixes CVE-2023-28464.

Link: https://syzkaller.appspot.com/bug?id=1bb51491ca5df96a5f724899d1dbb87afda61419 [1]

Signed-off-by: ZhengHan Wang <[email protected]>
Co-developed-by: Luiz Augusto von Dentz <[email protected]>
---

Re-sending Luiz' version of ZhengHan Wang's patch as a reply-to snafu
caused v2 of the patch by Luiz to not be applied.

I added the CVE reference, fixed the SoB and add mention of Luiz' changes
in the commit message.

net/bluetooth/hci_conn.c | 6 ++----
net/bluetooth/hci_sysfs.c | 23 ++++++++++++-----------
2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 7a6f20338db8..236085451931 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -169,13 +169,11 @@ static void hci_conn_cleanup(struct hci_conn *conn)
hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
}

- hci_conn_del_sysfs(conn);
-
debugfs_remove_recursive(conn->debugfs);

- hci_dev_put(hdev);
+ hci_conn_del_sysfs(conn);

- hci_conn_put(conn);
+ hci_dev_put(hdev);
}

static void hci_acl_create_connection(struct hci_conn *conn)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 15b33579007c..367e32fe30eb 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -35,7 +35,7 @@ void hci_conn_init_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;

- BT_DBG("conn %p", conn);
+ bt_dev_dbg(hdev, "conn %p", conn);

conn->dev.type = &bt_link;
conn->dev.class = &bt_class;
@@ -48,27 +48,30 @@ void hci_conn_add_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;

- BT_DBG("conn %p", conn);
+ bt_dev_dbg(hdev, "conn %p", conn);

if (device_is_registered(&conn->dev))
return;

dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle);

- if (device_add(&conn->dev) < 0) {
+ if (device_add(&conn->dev) < 0)
bt_dev_err(hdev, "failed to register connection device");
- return;
- }
-
- hci_dev_hold(hdev);
}

void hci_conn_del_sysfs(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;

- if (!device_is_registered(&conn->dev))
+ bt_dev_dbg(hdev, "conn %p", conn);
+
+ if (!device_is_registered(&conn->dev)) {
+ /* If device_add() has *not* succeeded, use *only* put_device()
+ * to drop the reference count.
+ */
+ put_device(&conn->dev);
return;
+ }

while (1) {
struct device *dev;
@@ -80,9 +83,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn)
put_device(dev);
}

- device_del(&conn->dev);
-
- hci_dev_put(hdev);
+ device_unregister(&conn->dev);
}

static void bt_host_release(struct device *dev)
--
2.41.0


2023-10-18 11:03:28

by bluez.test.bot

[permalink] [raw]
Subject: RE: [v3] Bluetooth: Fix double free in hci_conn_cleanup

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=794282

---Test result---

Test Summary:
CheckPatch FAIL 1.13 seconds
GitLint FAIL 0.63 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel PASS 33.67 seconds
CheckAllWarning PASS 36.66 seconds
CheckSparse PASS 42.12 seconds
CheckSmatch PASS 113.99 seconds
BuildKernel32 PASS 32.41 seconds
TestRunnerSetup PASS 510.44 seconds
TestRunner_l2cap-tester PASS 30.52 seconds
TestRunner_iso-tester PASS 50.01 seconds
TestRunner_bnep-tester PASS 10.43 seconds
TestRunner_mgmt-tester PASS 216.55 seconds
TestRunner_rfcomm-tester PASS 15.86 seconds
TestRunner_sco-tester PASS 19.64 seconds
TestRunner_ioctl-tester PASS 18.10 seconds
TestRunner_mesh-tester PASS 13.59 seconds
TestRunner_smp-tester PASS 14.38 seconds
TestRunner_userchan-tester PASS 11.50 seconds
IncrementalBuild PASS 33.41 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[v3] Bluetooth: Fix double free in hci_conn_cleanup
WARNING: Co-developed-by: must be immediately followed by Signed-off-by:
#83:
Co-developed-by: Luiz Augusto von Dentz <[email protected]>
---

total: 0 errors, 1 warnings, 0 checks, 70 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13426836.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3] Bluetooth: Fix double free in hci_conn_cleanup

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
39: B1 Line exceeds max length (87>80): "Link: https://syzkaller.appspot.com/bug?id=1bb51491ca5df96a5f724899d1dbb87afda61419 [1]"


---
Regards,
Linux Bluetooth

2023-10-18 20:20:34

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH v3] Bluetooth: Fix double free in hci_conn_cleanup

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

On Wed, 18 Oct 2023 12:30:55 +0200 you wrote:
> From: ZhengHan Wang <[email protected]>
>
> syzbot reports a slab use-after-free in hci_conn_hash_flush [1].
> After releasing an object using hci_conn_del_sysfs in the
> hci_conn_cleanup function, releasing the same object again
> using the hci_dev_put and hci_conn_put functions causes a double free.
> Here's a simplified flow:
>
> [...]

Here is the summary with links:
- [v3] Bluetooth: Fix double free in hci_conn_cleanup
https://git.kernel.org/bluetooth/bluetooth-next/c/4706a126c330

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html