[PATCH v3] Bluetooth: MGMT: Serialize power-on with power-off completion

[Cen Zhang <zzzccc427@gmail.com>]

mgmt_set_powered_complete() must consume only its own SET_POWERED
command. __mgmt_power_off() frees that command before its destroy callback
runs, and mgmt_pending_valid() recognizes pending commands only by address.

The power-on handler can reuse the address, but it is not the only
allocator. A concurrent set_mesh() can also publish another pending command
at that address after close drops hdev->lock:

  Close request worker              Management socket
  --------------------              -----------------
  __mgmt_power_off(): free cmd
  hci_dev_close_sync(): unlock
                                    set_mesh(): allocate replacement
  completion: claim old address
                                    submission fails: remove/free cmd
  completion: access replacement

Serialize power-on publication with req_lock, which the request worker
holds through its destroy callback. Keep power-off cancellation outside
that mutex and keep the req_lock -> hdev->lock ordering.

Also hold hdev->lock while finding the pending SET_POWERED command,
checking its identity, and consuming it. An unrelated command at the old
address then fails the opcode-specific lookup without being unlinked.
Power-on cannot reuse a SET_POWERED address until the old callback drops
req_lock, and power-off allocates no command once the device is down.

Dequeue pending power requests before settings_rsp() frees their command
data. Their cancellation callback already returns without touching cmd.
An already-dequeued request can still be waiting for req_lock. Check its
opcode under mgmt_pending_lock before copying parameters as well, so it
cannot interpret an unrelated replacement command as power settings.

KASAN report as below:

        BUG: KASAN: slab-use-after-free in
    mgmt_set_powered_complete+0x450/0x4d0
    Read of size 2 at addr ffff88810921ab10 by task kworker/u17:0/500

    Workqueue: hci0 hci_cmd_sync_work
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? mgmt_set_powered_complete+0x450/0x4d0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? mgmt_set_powered_complete+0x450/0x4d0
     kasan_report+0xe0/0x110
     ? mgmt_set_powered_complete+0x450/0x4d0
     mgmt_set_powered_complete+0x450/0x4d0
     ? __pfx_mgmt_set_powered_complete+0x10/0x10
     hci_cmd_sync_work+0x207/0x450
     ? __pfx_mgmt_set_powered_complete+0x10/0x10
     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 531:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     mgmt_pending_new+0x60/0x230
     mgmt_pending_add+0x3b/0x180
     set_powered+0x2a8/0x7e0
     hci_sock_sendmsg+0x128b/0x22e0
     __sys_sendto+0x425/0x470
     __x64_sys_sendto+0xe5/0x1c0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 531:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     set_powered+0x336/0x7e0
     hci_sock_sendmsg+0x128b/0x22e0
     __sys_sendto+0x425/0x470
     __x64_sys_sendto+0xe5/0x1c0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f
    ==================================================================

Fixes: eec8d2bcc841 ("Bluetooth: Add support for set_powered management command")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
Changes in v3:
- Address cross-opcode pending-command reuse reported by Sashiko:
  https://sashiko.dev/#/patchset/88964
- Validate SET_POWERED identity before claiming it.
- Cancel queued power requests before freeing their command data.
- Check the opcode of an already-dequeued request before reading its
  parameters.
- Retain the balanced power-on request locking introduced in v2.

Changes in v2:
- Move the common set_powered command path into a helper and acquire
  req_lock in the explicit power-on branch, fixing Clang context-analysis
  errors.
- Preserve req_lock -> hdev->lock ordering and keep power-off cancellation
  outside req_lock.
- Rebase onto current bluetooth-next.

Link to v1: https://lore.kernel.org/r/pm-bluetooth-objects-candidate-0063-v7-35a6680c94a332fea155@gmail.com
Link to v2: https://lore.kernel.org/r/pm-bluetooth-objects-candidate-0063-v11-a70581b6877e5fe118a3@gmail.com

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1342,9 +1342,15 @@
 	struct mgmt_pending_cmd *cmd = data;
 	struct mgmt_mode *cp;
 
-	/* Make sure cmd still outstanding. */
-	if (err == -ECANCELED || !mgmt_pending_valid(hdev, cmd))
+	if (err == -ECANCELED)
 		return;
+
+	hci_dev_lock(hdev);
+
+	/* Other management commands may reuse the old command address. */
+	if (pending_find(MGMT_OP_SET_POWERED, hdev) != cmd ||
+	    !mgmt_pending_valid(hdev, cmd))
+		goto unlock;
 
 	cp = cmd->param;
 
@@ -1352,10 +1358,8 @@
 
 	if (!err) {
 		if (cp->val) {
-			hci_dev_lock(hdev);
 			restart_le_actions(hdev);
 			hci_update_passive_scan(hdev);
-			hci_dev_unlock(hdev);
 		}
 
 		send_settings_rsp(cmd->sk, cmd->opcode, hdev);
@@ -1371,6 +1375,9 @@
 	}
 
 	mgmt_pending_free(cmd);
+
+unlock:
+	hci_dev_unlock(hdev);
 }
 
 static int set_powered_sync(struct hci_dev *hdev, void *data)
@@ -1380,8 +1387,9 @@
 
 	mutex_lock(&hdev->mgmt_pending_lock);
 
-	/* Make sure cmd still outstanding. */
-	if (!__mgmt_pending_listed(hdev, cmd)) {
+	/* A dequeued request may have waited across a controller reset. */
+	if (!__mgmt_pending_listed(hdev, cmd) ||
+	    cmd->opcode != MGMT_OP_SET_POWERED) {
 		mutex_unlock(&hdev->mgmt_pending_lock);
 		return -ECANCELED;
 	}
@@ -1395,18 +1403,12 @@
 	return hci_set_powered_sync(hdev, cp.val);
 }
 
-static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
-		       u16 len)
+static int __set_powered(struct sock *sk, struct hci_dev *hdev,
+			 void *data, u16 len)
 {
 	struct mgmt_mode *cp = data;
 	struct mgmt_pending_cmd *cmd;
 	int err;
-
-	bt_dev_dbg(hdev, "sock %p", sk);
-
-	if (cp->val != 0x00 && cp->val != 0x01)
-		return mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
-				       MGMT_STATUS_INVALID_PARAMS);
 
 	hci_dev_lock(hdev);
 
@@ -1454,6 +1456,32 @@
 	return err;
 }
 
+static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
+		       u16 len)
+{
+	struct mgmt_mode *cp = data;
+	int err;
+
+	bt_dev_dbg(hdev, "sock %p", sk);
+
+	if (cp->val != 0x00 && cp->val != 0x01)
+		return mgmt_cmd_status(sk, hdev->id, MGMT_OP_SET_POWERED,
+				       MGMT_STATUS_INVALID_PARAMS);
+
+	if (!cp->val)
+		return __set_powered(sk, hdev, data, len);
+
+	/*
+	 * Wait for the previous power-off completion before publishing a
+	 * new command that could reuse its freed pending-command address.
+	 */
+	hci_req_sync_lock(hdev);
+	err = __set_powered(sk, hdev, data, len);
+	hci_req_sync_unlock(hdev);
+
+	return err;
+}
+
 int mgmt_new_settings(struct hci_dev *hdev)
 {
 	return new_settings(hdev, NULL);
@@ -1468,6 +1496,11 @@
 static void settings_rsp(struct mgmt_pending_cmd *cmd, void *data)
 {
 	struct cmd_lookup *match = data;
+
+	/* A queued power request must not retain the freed command. */
+	if (cmd->opcode == MGMT_OP_SET_POWERED)
+		hci_cmd_sync_dequeue(match->hdev, set_powered_sync, cmd,
+				     mgmt_set_powered_complete);
 
 	send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);