Re: [PATCH] Bluetooth: Fix double free in hci_conn_cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Do you think this might be the cause of https://bugzilla.kernel.org/show_bug.cgi?id=201269 ?

Jay

On 3/30/23 3:02 PM, Luiz Augusto von Dentz wrote:
From: ZhengHan Wang <wzhmmmmm@xxxxxxxxx>

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.

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

Signed-off-by: ZhengHan Wang <wzhmmmmm@xxxxxxxxx>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
---
  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 e4aee5950c36..00d1e7201a44 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -168,13 +168,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 le_scan_cleanup(struct work_struct *work)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 08542dfc2dc5..633b82d54272 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)



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux