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

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

[Cen Zhang]

mgmt_set_powered_complete() must only consume the pending command for
its own request. However, __mgmt_power_off() removes and frees the
SET_POWERED command before the request's destroy callback runs, and
mgmt_pending_valid() identifies outstanding commands only by address.

When a powered controller is switched off and another management socket
requests power-on, the allocator can reuse the old command's address:

  Power-off request worker          Power-on management handler
  ------------------------          ---------------------------
  __mgmt_power_off(): free cmd
  hci_dev_close_sync(): unlock
  set_powered_sync(): return
                                    set_powered(): lock hdev
                                    allocate and link replacement
  mgmt_pending_valid(): unlink
    replacement at old address
  read replacement's cp->val = 1
  wait for hdev->lock
                                    submit fails: remove/free cmd
                                    unlock hdev
  acquire hdev->lock
  read freed cmd->opcode

The old callback can respond through and free the new command instead of
completing its own request. If hci_cmd_sync_submit() fails to allocate
its work entry after the callback has unlinked the replacement,
set_powered() removes and frees it again while still holding hdev->lock.
The old callback then acquires that lock and reads the freed command.

Serialize power-on publication with the existing request mutex, which
hci_cmd_sync_work() holds across the request function and its destroy
callback. Hold it through submission and error cleanup so the
replacement cannot be published before the old callback finishes.
Acquire it before hdev->lock to preserve the worker's lock order.
Power-off retains its existing cancellation path.

Put the shared command path in a helper and acquire the request mutex
unconditionally around the power-on call. Keeping acquisition and
release in the same control-flow path also allows Clang context analysis
to verify balanced locking.

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

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3b9c82bd2f027..90686ebec2399 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1395,19 +1395,13 @@ static int set_powered_sync(struct hci_dev *hdev, void *data)
 	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);
 
 	if (!cp->val) {
@@ -1454,6 +1448,32 @@ static int set_powered(struct sock *sk, struct hci_dev *hdev, void *data,
 	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);

RE: [v2] 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/1186652/

---Test result---

Test Summary:
CheckPatch                    PASS      0.57 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       PASS      0.22 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      23.35 seconds
CheckAllWarning               PASS      27.23 seconds
CheckSparse                   PASS      29.93 seconds
BuildKernel32                 PASS      25.12 seconds
CheckKernelLLVM               PASS      28.52 seconds
TestRunnerSetup               PASS      530.21 seconds
TestRunner_mgmt-tester        PASS      83.49 seconds
TestRunner_mesh-tester        PASS      5.15 seconds
IncrementalBuild              PASS      22.36 seconds



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

---
Regards,
Linux Bluetooth