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

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

[Cen Zhang]

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);
 

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

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1191273/

---Test result---

Test Summary:
CheckPatch                    PASS      5.70 seconds
VerifyFixes                   PASS      0.14 seconds
VerifySignedoff               PASS      0.09 seconds
GitLint                       PASS      0.26 seconds
SubjectPrefix                 PASS      0.11 seconds
BuildKernel                   PASS      28.86 seconds
CheckAllWarning               PASS      33.45 seconds
CheckSparse                   PASS      35.24 seconds
BuildKernel32                 PASS      29.26 seconds
CheckKernelLLVM               PASS      29.80 seconds
TestRunnerSetup               PASS      679.97 seconds
TestRunner_mgmt-tester        PASS      87.48 seconds
TestRunner_mesh-tester        PASS      5.25 seconds
IncrementalBuild              PASS      27.19 seconds



https://github.com/bluez/bluetooth-next/pull/979

---
Regards,
Linux Bluetooth