github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-05-26 16:12:59 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths

Browse Source 
Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and
write_work) are only flushed/cancelled if the HCI_UART_PROTO_READY
flag is set during TTY close. If a hangup occurs before setup completes,
hci_uart_tty_close() skips the teardown of these workqueues and
proceeds to free the `hu` struct. When the scheduled work executes
later, it blindly dereferences the freed `hu` struct.

Furthermore, several data races and UAFs were identified in the teardown
sequence:
1. Calling hci_uart_flush() from hci_uart_close() without effectively
   disabling write_work causes a race condition where both can concurrently
   double-free hu->tx_skb. This happens because protocol timers can
   concurrently invoke hci_uart_tx_wakeup() and requeue write_work.
2. Calling hci_free_dev(hdev) before hu->proto->close(hu) causes a UAF
   when vendor specific protocol close callbacks dereference hu->hdev.
3. In the initialization error paths, failing to take the proto_lock
   write lock before clearing PROTO_READY leads to races with active
   readers. Additionally, hci_uart_tty_receive() accesses hu->hdev
   outside the read lock, leading to UAFs if the initialization error
   path frees hdev concurrently.

Fix these synchronization and lifecycle issues by:
1. Re-ordering hci_uart_tty_close() to clear HCI_UART_PROTO_READY first,
   followed immediately by a cancel_work_sync(&hu->write_work). Clearing
   the flag locks out concurrent protocol timers from successfully invoking
   hci_uart_tx_wakeup(), effectively rendering the cancellation permanent
   and preventing the tx_skb double-free.
2. Note: Clearing PROTO_READY early causes hci_uart_close() to skip
   hu->proto->flush(). This is perfectly safe in the tty_close path
   because hu->proto->close() executes shortly after, which intrinsically
   purges all protocol SKB queues and tears down the state.
3. Relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev)
   across all close and error paths to prevent vendor-level UAFs.
4. Moving the hdev->stat.byte_rx increment in hci_uart_tty_receive()
   inside the proto_lock read-side critical section to safely synchronize
   with device unregistration.
5. Adding cancel_work_sync(&hu->write_work) to hci_uart_close() to safely
   flush the workqueue before hci_uart_flush() is invoked via the HCI core.
6. Utilizing cancel_work_sync() instead of disable_work_sync() across
   all paths to prevent permanently breaking user-space retry capabilities.

Fixes: 3b799254cf ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This commit is contained in:
Mingyu Wang 2026-05-18 10:49:49 +08:00 committed by Luiz Augusto von Dentz
parent d3f7d17960
commit c1bb9336ae
1 changed files with 40 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
drivers/bluetooth/hci_ldisc.c
View File

@ -194,7 +194,15 @@ void hci_uart_init_work(struct work_struct *work)
err = hci_register_dev(hu->hdev);
if (err < 0) {
BT_ERR("Can't register HCI device");
percpu_down_write(&hu->proto_lock);
clear_bit(HCI_UART_PROTO_READY, &hu->flags);
percpu_up_write(&hu->proto_lock);
/* Safely cancel work after clearing flags */
cancel_work_sync(&hu->write_work);
/* Close protocol before freeing hdev */
hu->proto->close(hu);
hdev = hu->hdev;
hu->hdev = NULL;
@ -263,8 +271,12 @@ static int hci_uart_open(struct hci_dev *hdev)
/* Close device */
static int hci_uart_close(struct hci_dev *hdev)
{
struct hci_uart *hu = hci_get_drvdata(hdev);
BT_DBG("hdev %p", hdev);
cancel_work_sync(&hu->write_work);
hci_uart_flush(hdev);
hdev->flush = NULL;
return 0;
@ -531,6 +543,7 @@ static void hci_uart_tty_close(struct tty_struct *tty)
{
struct hci_uart *hu = tty->disc_data;
struct hci_dev *hdev;
bool proto_ready;
BT_DBG("tty %p", tty);
@ -540,24 +553,38 @@ static void hci_uart_tty_close(struct tty_struct *tty)
if (!hu)
return;
hdev = hu->hdev;
if (hdev)
hci_uart_close(hdev);
/* Wait for init_ready to finish to prevent registration races */
cancel_work_sync(&hu->init_ready);
if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
if (proto_ready) {
percpu_down_write(&hu->proto_lock);
clear_bit(HCI_UART_PROTO_READY, &hu->flags);
percpu_up_write(&hu->proto_lock);
}
cancel_work_sync(&hu->init_ready);
cancel_work_sync(&hu->write_work);
/*
* Unconditionally cancel write_work AFTER clearing PROTO_READY.
* This ensures that concurrent protocol timers cannot requeue
* write_work via hci_uart_tx_wakeup(), permanently preventing
* double-free races and UAFs.
*/
cancel_work_sync(&hu->write_work);
hdev = hu->hdev;
if (hdev)
hci_uart_close(hdev); /* proto->flush is safely skipped */
if (proto_ready) {
if (hdev) {
if (test_bit(HCI_UART_REGISTERED, &hu->flags))
hci_unregister_dev(hdev);
hci_free_dev(hdev);
}
/* Close protocol before freeing hdev (intrinsically purges queues) */
hu->proto->close(hu);
if (hdev)
hci_free_dev(hdev);
}
clear_bit(HCI_UART_PROTO_SET, &hu->flags);
@ -625,11 +652,12 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
* tty caller
*/
hu->proto->recv(hu, data, count);
percpu_up_read(&hu->proto_lock);
if (hu->hdev)
hu->hdev->stat.byte_rx += count;
percpu_up_read(&hu->proto_lock);
tty_unthrottle(tty);
}
@ -695,6 +723,10 @@ static int hci_uart_register_dev(struct hci_uart *hu)
percpu_down_write(&hu->proto_lock);
clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
percpu_up_write(&hu->proto_lock);
/* Cancel work after clearing flags */
cancel_work_sync(&hu->write_work);
/* Close protocol before freeing hdev */
hu->proto->close(hu);
hu->hdev = NULL;
hci_free_dev(hdev);