Forwarded: [PATCH] Bluetooth: hci_core: Fix SRCU leak when device is freed unregistered

[syzbot <syzbot+535ecc844591e50588a5@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] Bluetooth: hci_core: Fix SRCU leak when device is freed unregistered
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master



hci_alloc_dev_priv() initializes hdev->srcu with init_srcu_struct(), but
the matching cleanup_srcu_struct() is only called from hci_unregister_dev().
A hci_dev that is allocated and then freed without ever being registered
leaks the SRCU internals - the node array and the per-CPU sda - even though
the hci_dev itself is freed correctly.

This is reachable from the hci_uart line discipline. h5_open() (used by the
HCI_UART_3WIRE protocol) sets HCI_UART_INIT_PENDING, so hci_uart_register_dev()
returns early without calling hci_register_dev() and HCI_UART_REGISTERED is
never set. Registration is deferred until the three-wire link is synced, which
never happens over a bare pty. When the tty is closed, hci_uart_tty_close()
finds HCI_UART_REGISTERED clear, skips hci_unregister_dev(), and calls
hci_free_dev() -> hci_release_dev() -> kfree(hdev), orphaning the SRCU
allocations.

kmemleak reports only the SRCU sub-objects, not hdev, confirming that hdev is
freed while its embedded srcu is not torn down:

  init_srcu_struct_fields+0x2c0/0x350 kernel/rcu/srcutree.c:207
  hci_alloc_dev_priv+0x37/0x680 net/bluetooth/hci_core.c:2453
  hci_alloc_dev include/net/bluetooth/hci_core.h:1763 [inline]
  hci_uart_register_dev drivers/bluetooth/hci_ldisc.c:644 [inline]
  hci_uart_set_proto drivers/bluetooth/hci_ldisc.c:720 [inline]
  hci_uart_tty_ioctl+0x173/0x460 drivers/bluetooth/hci_ldisc.c:774

Pair the SRCU init with destruction by calling cleanup_srcu_struct() from
hci_release_dev(), so it runs on the final put_device() regardless of whether
the device was ever registered. Keep synchronize_srcu() in hci_unregister_dev()
to drain readers walking the device list before the device leaves it.

Reported-by: syzbot+535ecc844591e50588a5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=535ecc844591e50588a5
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 net/bluetooth/hci_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 28d7929dc593..2d516beedb59 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2664,7 +2664,6 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	write_unlock(&hci_dev_list_lock);
 
 	synchronize_srcu(&hdev->srcu);
-	cleanup_srcu_struct(&hdev->srcu);
 
 	disable_work_sync(&hdev->rx_work);
 	disable_work_sync(&hdev->cmd_work);
@@ -2737,6 +2736,8 @@ void hci_release_dev(struct hci_dev *hdev)
 	kfree_skb(hdev->sent_cmd);
 	kfree_skb(hdev->req_skb);
 	kfree_skb(hdev->recv_event);
+
+	cleanup_srcu_struct(&hdev->srcu);
 	kfree(hdev);
 }
 EXPORT_SYMBOL(hci_release_dev);
-- 
2.43.0