[PATCH v2] Bluetooth: L2CAP: Protect the receive reject-list lookup

[Cen Zhang <zzzccc427@gmail.com>]

Reject-list entries must remain alive throughout a lookup. Management
updates the list under hdev->lock, but l2cap_recv_frame() traverses it
under conn->lock after l2cap_recv_acldata() has released hdev->lock.

An established LE connection can still receive ACL data after its peer
has been added to the reject list. If a management socket unblocks that
peer during the receive-side lookup, the following ordering is possible:

  HCI receive                        Management
  l2cap_recv_acldata()
    release hdev->lock
    acquire conn->lock
  l2cap_recv_frame()
    lookup selects entry
                                     unblock_device()
                                       acquire hdev->lock
                                       hci_bdaddr_list_del()
                                         unlink and kfree(entry)
    read entry->bdaddr_type

Management does not take conn->lock, so deletion can free the selected
entry before the lookup reads its fields. This causes a use-after-free
read in hci_bdaddr_list_lookup().

Compute the reject decision under the existing device lock in
l2cap_recv_acldata() and pass only the boolean to l2cap_recv_frame().
For pending receive data, take the snapshot in l2cap_conn_ready() and
l2cap_security_cfm(), which already hold hdev->lock. Publish it under
conn->lock before queuing the worker on the existing device workqueue:

  HCI callback                       Pending RX worker
    hold hdev->lock
    acquire conn->lock
    lookup reject list
    save boolean; queue work
    release conn->lock
                                       acquire conn->lock
                                       consume boolean
                                       drain pending frames

The snapshot belongs to the scheduled batch. A subsequent scheduling
callback refreshes it under both locks, including when queue_work()
coalesces with already pending work. No reject-list pointer survives the
lookup, and the worker reads the boolean under conn->lock.

Taking the device lock in the worker would deadlock against
l2cap_conn_del(), which holds it while cancelling the work synchronously.
A failed trylock followed by immediate self-requeue would instead keep
retrying without a delay and extend a concurrent workqueue drain. Keep
the lookup in the scheduling callbacks so the worker needs neither the
device lock nor a retry. Skip scheduling when pending_rx is empty.

KASAN report as below:

    ==================================================================
    BUG: KASAN: slab-use-after-free in hci_bdaddr_list_lookup+0x145/0x160
    Read of size 1 at addr ffff88810cba64d6 by task kworker/u17:0/493

    Workqueue: hci0 hci_rx_work
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? hci_bdaddr_list_lookup+0x145/0x160
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? hci_bdaddr_list_lookup+0x145/0x160
     kasan_report+0xe0/0x110
     ? hci_bdaddr_list_lookup+0x145/0x160
     hci_bdaddr_list_lookup+0x145/0x160
     l2cap_recv_frame+0x865/0x8830
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __mutex_lock+0x182c/0x2590
     ? __pfx_l2cap_recv_frame+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? find_held_lock+0x2b/0x80
     ? l2cap_recv_acldata+0x1eb/0x10e0
     ? __pfx___mutex_lock+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __mutex_unlock_slowpath+0x147/0x920
     ? __pfx_l2cap_conn_hold_unless_zero+0x10/0x10
     ? __pfx_hci_conn_enter_active_mode+0x10/0x10
     ? l2cap_recv_acldata+0x13d/0x10e0
     ? srso_alias_return_thunk+0x5/0xfbef5
     l2cap_recv_acldata+0xe9f/0x10e0
     ? srso_alias_return_thunk+0x5/0xfbef5
     hci_rx_work+0x5a6/0xf00
     ? srso_alias_return_thunk+0x5/0xfbef5
     process_one_work+0x908/0x19c0
     ? __pfx_process_one_work+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_is_held_type+0x8f/0x100
     ? srso_alias_return_thunk+0x5/0xfbef5
     worker_thread+0x65c/0xe40
     ? __pfx_worker_thread+0x10/0x10
     kthread+0x34f/0x460
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __pfx_kthread+0x10/0x10
     ret_from_fork+0x659/0x940
     ? __pfx_ret_from_fork+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __switch_to+0x74f/0xf80
     ? __pfx_kthread+0x10/0x10
     ret_from_fork_asm+0x1a/0x30
     </TASK>

    Allocated by task 604:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     hci_bdaddr_list_add+0xdf/0x2c0
     block_device+0x112/0x260
     hci_sock_sendmsg+0x128b/0x22e0
     sock_write_iter+0x4c0/0x550
     do_iter_readv_writev+0x59c/0x860
     vfs_writev+0x30e/0xc10
     do_writev+0x241/0x2f0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 606:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     hci_bdaddr_list_del+0x1a9/0x2a0
     unblock_device+0x112/0x260
     hci_sock_sendmsg+0x128b/0x22e0
     sock_write_iter+0x4c0/0x550
     do_iter_readv_writev+0x59c/0x860
     vfs_writev+0x30e/0xc10
     do_writev+0x241/0x2f0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    The buggy address belongs to the object at ffff88810cba64c0
                                                                which
                                belongs to the cache kmalloc-32 of size 32
    The buggy address is located 22 bytes inside of
                                                                freed
                                32-byte region [ffff88810cba64c0,
                                ffff88810cba64e0)

    The buggy address belongs to the physical page:
        page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
    pfn:0x10cba6
    flags: 0x200000000000000(node=0|zone=2)
    page_type: f5(slab)
        raw: 0200000000000000 ffff888100042780 dead000000000100
    dead000000000122
        raw: 0000000000000000 0000000000400040 00000000f5000000
    0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
     ffff88810cba6380: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
     ffff88810cba6400: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
    >ffff88810cba6480: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
                                                     ^
     ffff88810cba6500: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
     ffff88810cba6580: fa fb fb fb fc fc fc fc fa fb fb fb fc fc fc fc
    ==================================================================

Fixes: cc8dba2bc302 ("Bluetooth: Block ATT connection on LE when device is blocked")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Changes in v2:
- Address the immediate self-requeue concern raised by Sashiko:
  https://sashiko.dev/#/patchset/95508
- Snapshot reject-list membership under hdev->lock in the connection and
  security callbacks, and pass the decision to pending RX under conn->lock.
- Remove the worker trylock and self-requeue, and avoid scheduling an empty
  pending RX queue.

Link to v1: https://lore.kernel.org/r/pm-bluetooth-objects-candidate-0157-v7-408aa9db5365c34b5f8b@gmail.com

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 54eddcdd341f9..bca4a16c99b54 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -636,6 +636,7 @@ struct l2cap_conn {
 
 	struct sk_buff_head	pending_rx;
 	struct work_struct	pending_rx_work;
+	bool			pending_rx_blocked __guarded_by(&lock);
 
 	struct delayed_work	id_addr_timer;
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 83b622a428118..fda25fe726c26 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1759,10 +1759,40 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	}
 }
 
-static void l2cap_conn_ready(struct l2cap_conn *conn)
+static bool l2cap_is_blocked(struct hci_conn *hcon)
+	__must_hold(&hcon->hdev->lock)
+{
+	/* Since we can't actively block incoming LE connections we must
+	 * at least ensure that we ignore incoming data from them.
+	 */
+	return hcon->type == LE_LINK &&
+	       hci_bdaddr_list_lookup(&hcon->hdev->reject_list, &hcon->dst,
+				      bdaddr_dst_type(hcon));
+}
+
+static void l2cap_queue_pending_rx(struct hci_conn *hcon)
+	__must_hold(&hcon->hdev->lock)
 {
+	struct l2cap_conn *conn = hcon->l2cap_data;
+
+	mutex_lock(&conn->lock);
+
+	if (!skb_queue_empty(&conn->pending_rx)) {
+		/* Teardown holds hdev->lock while cancelling the worker.
+		 * Pass the decision under conn->lock to avoid that lock.
+		 */
+		conn->pending_rx_blocked = l2cap_is_blocked(hcon);
+		queue_work(hcon->hdev->workqueue, &conn->pending_rx_work);
+	}
+
+	mutex_unlock(&conn->lock);
+}
+
+static void l2cap_conn_ready(struct hci_conn *hcon)
+	__must_hold(&hcon->hdev->lock)
+{
+	struct l2cap_conn *conn = hcon->l2cap_data;
 	struct l2cap_chan *chan;
-	struct hci_conn *hcon = conn->hcon;
 
 	BT_DBG("conn %p", conn);
 
@@ -1794,7 +1824,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 	if (hcon->type == LE_LINK)
 		l2cap_le_conn_ready(conn);
 
-	queue_work(hcon->hdev->workqueue, &conn->pending_rx_work);
+	l2cap_queue_pending_rx(hcon);
 }
 
 /* Notify sockets that we cannot guaranty reliability anymore */
@@ -7381,7 +7411,8 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
 	kfree_skb(skb);
 }
 
-static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
+static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb,
+			     bool blocked)
 	__must_hold(&conn->lock)
 {
 	struct l2cap_hdr *lh = (void *) skb->data;
@@ -7408,12 +7439,7 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
 		return;
 	}
 
-	/* Since we can't actively block incoming LE connections we must
-	 * at least ensure that we ignore incoming data from them.
-	 */
-	if (hcon->type == LE_LINK &&
-	    hci_bdaddr_list_lookup(&hcon->hdev->reject_list, &hcon->dst,
-				   bdaddr_dst_type(hcon))) {
+	if (blocked) {
 		kfree_skb(skb);
 		return;
 	}
@@ -7461,7 +7487,7 @@ static void process_pending_rx(struct work_struct *work)
 	 */
 	while (!test_bit(HCI_CONN_ENC_KEY_SIZE_PEND, conn->hcon->flags) &&
 	       (skb = skb_dequeue(&conn->pending_rx)))
-		l2cap_recv_frame(conn, skb);
+		l2cap_recv_frame(conn, skb, conn->pending_rx_blocked);
 
 	mutex_unlock(&conn->lock);
 }
@@ -7912,7 +7938,7 @@ static void l2cap_connect_cfm(struct hci_conn *hcon, u8 status)
 
 	mutex_unlock(&conn->lock);
 
-	l2cap_conn_ready(conn);
+	l2cap_conn_ready(hcon);
 }
 
 int l2cap_disconn_ind(struct hci_conn *hcon)
@@ -8051,8 +8077,7 @@ static void l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 	mutex_unlock(&conn->lock);
 
 	/* Process data held back while the encryption key size was read */
-	if (!skb_queue_empty(&conn->pending_rx))
-		queue_work(hcon->hdev->workqueue, &conn->pending_rx_work);
+	l2cap_queue_pending_rx(hcon);
 }
 
 /* Append fragment into frame respecting the maximum len of rx_skb */
@@ -8141,6 +8166,7 @@ int l2cap_recv_acldata(struct hci_dev *hdev, u16 handle,
 {
 	struct hci_conn *hcon;
 	struct l2cap_conn *conn;
+	bool blocked;
 	int len;
 
 	/* Lock hdev for hci_conn, and race on l2cap_data vs. l2cap_conn_del */
@@ -8163,6 +8189,7 @@ int l2cap_recv_acldata(struct hci_dev *hdev, u16 handle,
 		conn = l2cap_conn_add(hcon);
 
 	conn = l2cap_conn_hold_unless_zero(conn);
+	blocked = l2cap_is_blocked(hcon);
 	hcon = NULL;
 
 	hci_dev_unlock(hdev);
@@ -8199,7 +8226,7 @@ int l2cap_recv_acldata(struct hci_dev *hdev, u16 handle,
 
 		if (len == skb->len) {
 			/* Complete frame received */
-			l2cap_recv_frame(conn, skb);
+			l2cap_recv_frame(conn, skb, blocked);
 			goto unlock;
 		}
 
@@ -8223,7 +8250,7 @@ int l2cap_recv_acldata(struct hci_dev *hdev, u16 handle,
 			 * setting the socket unreliable.
 			 */
 			skb->len = len;
-			l2cap_recv_frame(conn, skb);
+			l2cap_recv_frame(conn, skb, blocked);
 			l2cap_conn_unreliable(conn, ECOMM);
 			goto unlock;
 		}
@@ -8273,7 +8300,7 @@ int l2cap_recv_acldata(struct hci_dev *hdev, u16 handle,
 			 */
 			struct sk_buff *rx_skb = conn->rx_skb;
 			conn->rx_skb = NULL;
-			l2cap_recv_frame(conn, rx_skb);
+			l2cap_recv_frame(conn, rx_skb, blocked);
 		}
 		break;
 	}