From: ZhengHan Wang wzhmmmmm@gmail.com
[ Upstream commit a85fb91e3d728bdfc80833167e8162cce8bc7004 ]
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=1bb51491ca5df96a5f724899d1dbb87afda6141... [1]
Signed-off-by: ZhengHan Wang wzhmmmmm@gmail.com 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: Sasha Levin sashal@kernel.org --- 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 b876e97b61c92..0e837feaa527e 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -125,13 +125,11 @@ static void hci_conn_cleanup(struct hci_conn *conn) if (hdev->notify) 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 le_scan_cleanup(struct work_struct *work) diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index ccd2c377bf83c..266112c960ee8 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -33,7 +33,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; @@ -46,27 +46,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; @@ -78,9 +81,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)