Skip to content

Commit

Permalink
Bluetooth: Fix double free in hci_conn_cleanup
Browse files Browse the repository at this point in the history
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 <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>
  • Loading branch information
unrav31 authored and Vudentz committed Oct 23, 2023
1 parent 4ed924f commit a85fb91
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 15 deletions.
6 changes: 2 additions & 4 deletions net/bluetooth/hci_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,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)
Expand Down
23 changes: 12 additions & 11 deletions net/bluetooth/hci_sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)
Expand Down

0 comments on commit a85fb91

Please sign in to comment.