Re: [PATCH] Bluetooth: MGMT: dequeue queued mesh send before freeing canceled tx
Ali Ahmet Memis <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth |
|---|---|
| Message-ID | <[email protected]> |
Hi Cen,
On Fri, Jul 3, 2026 at 14:00, Cen Zhang wrote:
> Dequeue any queued mesh_send_sync() for the target tx from send_cancel().
The analysis looks right to me, but the fix is scoped to send_cancel()
and there is a second path that frees a mgmt_mesh_tx while a queued
mesh_send_sync() still carries the pointer.
mgmt_cleanup(), called from hci_sock_release(), walks every controller
and completes the mesh entries belonging to the closing socket:
list_for_each_entry(hdev, &hci_dev_list, list) {
do {
mesh_tx = mgmt_mesh_next(hdev, sk);
if (mesh_tx)
mesh_send_complete(hdev, mesh_tx, true);
} while (mesh_tx);
}
mesh_send_complete() reaches mgmt_mesh_remove(), which does the same
list_del() + sock_put() + kfree() your commit message describes, with no
hci_cmd_sync_dequeue() first. So closing the control socket right after
MGMT_OP_MESH_SEND, or while mesh_next() has queued the send for the next
tx, frees the object the queued mesh_send_sync() will dereference. That
is the same use-after-free, reached without MGMT_OP_MESH_SEND_CANCEL.
Separately, mgmt_cleanup() holds only hci_dev_list_lock, never
hdev->lock, while it traverses hdev->mesh_pending and removes entries
from it. mesh_send() inserts into that list under hci_dev_lock(hdev), so
the list itself is walked unsynchronised against insertion. Note this
one cannot be fixed by simply adding hci_dev_lock() there, since
hci_dev_list_lock is an rwlock and hci_dev_lock() is a mutex; it needs
the usual hci_dev_hold() and drop-the-list-lock dance.
Given both, an explicit owner reference on mgmt_mesh_tx may end up
simpler than dequeuing at each free site: the pending list holds one
reference, the queued cmd_sync entry holds another via a destroy
callback, and only the last put does sock_put() and kfree(). That would
cover send_cancel() and mgmt_cleanup() together, and would not need
every future caller to remember the dequeue.
To be clear about what I did and did not do: this is from reading the
code, I have not reproduced the socket-close variant, and I have not
tested your patch.
Regards,
Ali Ahmet Memis