Re: [PATCH] Bluetooth: btusb: fix use-after-free in force_poll_sync

[YuanZhe Liu]

Hi Luiz,

Thanks for the review.

The vendor_diag observation is correct. For BTUSB_BCM_PATCHRAM and
BTUSB_BCM_APPLE, btusb_probe() sets hdev->set_diag = btusb_bcm_set_diag,
and hci_debugfs_create_basic() creates vendor_diag under hdev->debugfs.
btusb does not set HCI_QUIRK_NON_PERSISTENT_DIAG, so vendor_diag_write()
calls hdev->set_diag() even after the device has been unregistered, and
btusb_bcm_set_diag() reads btusb_data->diag before checking HCI_RUNNING.
As hdev->debugfs is only removed in hci_release_dev(), a write after
btusb_disconnect() can reach freed btusb_data.

That is a separate pre-existing issue: force_poll_sync is not the only
HCI debugfs file that can reach btusb_data, and the changelog sentence
claiming btusb_data can no longer be dereferenced was too broad. v3
scopes that guarantee to the force_poll_sync file.

For hci_qca, qca_debugfs_init() stores pointers to qca_data in the ibs
files and qca_close() frees qca_data without removing them. That is also
pre-existing but self-contained in the driver, so I sent a separate
patch that removes the ibs directory before qca_data is freed.

For vendor_diag, btusb cannot remove a file it does not own, so I think
the fix belongs in hci_debugfs: either remove the basic files in
hci_unregister_dev(), or make vendor_diag_write() reject writes once
HCI_UNREGISTER is set. I can prepare that as a follow-up if you agree.

Thanks,
YuanZhe