| Message ID | 20230330220235.1035456-1-luiz.dentz@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Bluetooth: Fix double free in hci_conn_cleanup | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | 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 34: B1 Line exceeds max length (87>80): "Link: https://syzkaller.appspot.com/bug?id=1bb51491ca5df96a5f724899d1dbb87afda61419 [1]" |
| 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 |
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=735596 ---Test result--- Test Summary: CheckPatch PASS 0.75 seconds GitLint FAIL 0.53 seconds SubjectPrefix PASS 0.09 seconds BuildKernel PASS 31.29 seconds CheckAllWarning PASS 34.91 seconds CheckSparse PASS 39.07 seconds CheckSmatch PASS 108.89 seconds BuildKernel32 PASS 30.34 seconds TestRunnerSetup PASS 435.71 seconds TestRunner_l2cap-tester PASS 16.15 seconds TestRunner_iso-tester PASS 15.63 seconds TestRunner_bnep-tester PASS 5.10 seconds TestRunner_mgmt-tester PASS 106.90 seconds TestRunner_rfcomm-tester PASS 8.21 seconds TestRunner_sco-tester PASS 7.51 seconds TestRunner_ioctl-tester PASS 8.72 seconds TestRunner_mesh-tester PASS 6.37 seconds TestRunner_smp-tester PASS 7.38 seconds TestRunner_userchan-tester PASS 5.29 seconds IncrementalBuild PASS 28.65 seconds Details ############################## Test: GitLint - FAIL Desc: Run gitlint Output: 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 34: B1 Line exceeds max length (87>80): "Link: https://syzkaller.appspot.com/bug?id=1bb51491ca5df96a5f724899d1dbb87afda61419 [1]" --- Regards, Linux Bluetooth
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@gmail.com> > > 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@gmail.com> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> > --- > 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)
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)