* [PATCH] Bluetooth: mgmt: copy pending command data under the list lock
@ 2026-06-18 10:21 Cen Zhang
2026-06-18 15:46 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Cen Zhang @ 2026-06-18 10:21 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz
Cc: linux-bluetooth, linux-kernel, baijiaju1990, zzzccc427
mgmt_pending_find() only protects the pending list lookup. Once it
returns, a caller that dereferences the returned command has no lifetime
guarantee unless another lock or ownership transfer keeps the command from
being removed and freed.
mgmt_powering_down() only needs the requested SET_POWERED mode, but it
currently keeps the raw pending command pointer after the list lock has
been dropped and then reads cmd->param and cp->val.
The buggy scenario involves two paths, with each column showing the order
within that path:
hci_suspend_dev()/hci_resume_dev(): SET_POWERED completion:
1. mgmt_powering_down() calls 1. mgmt_set_powered_complete()
pending_find()
2. pending_find() drops 2. mgmt_pending_valid() delists
mgmt_pending_lock the command
3. mgmt_pending_free() frees the
command and parameter buffer
3. mgmt_powering_down() reads
cmd->param
4. mgmt_powering_down() reads
cp->val
Add a helper that copies a requested pending-command parameter slice while
mgmt_pending_lock is still held. Use it for the mgmt_mode readers in
mgmt.c so they take a snapshot of the pending request instead of keeping an
unlocked raw command pointer just to inspect the requested mode.
Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in mgmt_powering_down+0xa0/0xf0
Call Trace:
<TASK>
dump_stack_lvl+0x66/0xa0
print_report+0xce/0x5f0
? mgmt_powering_down+0xa0/0xf0
? srso_alias_return_thunk+0x5/0xfbef5
? __virt_addr_valid+0x19f/0x330
? mgmt_powering_down+0xa0/0xf0
kasan_report+0xe0/0x110
? mgmt_powering_down+0xa0/0xf0
mgmt_powering_down+0xa0/0xf0
hci_suspend_dev+0xc0/0x2d0
? vhci_suspend_work+0x31/0x50
process_one_work+0x4fd/0xbc0
? __pfx_process_one_work+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? srso_alias_return_thunk+0x5/0xfbef5
? __list_add_valid_or_report+0x37/0xf0
? __pfx_vhci_suspend_work+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
worker_thread+0x2d8/0x570
? srso_alias_return_thunk+0x5/0xfbef5
? __pfx_worker_thread+0x10/0x10
kthread+0x1ad/0x1f0
? __pfx_kthread+0x10/0x10
ret_from_fork+0x3c9/0x540
? __pfx_ret_from_fork+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? __switch_to+0x2e9/0x730
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1a/0x30
</TASK>
Allocated by task 336 on cpu 2 at 98.764916s:
kasan_save_stack+0x33/0x60
kasan_save_track+0x17/0x60
__kasan_kmalloc+0xaa/0xb0
mgmt_pending_new+0x44/0x130
mgmt_pending_add+0x22/0x110
set_powered+0x1ad/0x310
hci_sock_sendmsg+0x96b/0xf80
sock_write_iter+0x28e/0x2a0
do_iter_readv_writev+0x211/0x390
vfs_writev+0x266/0x7b0
do_writev+0x191/0x1d0
do_syscall_64+0x115/0x6a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Freed by task 314 on cpu 0 at 101.816391s:
kasan_save_stack+0x33/0x60
kasan_save_track+0x17/0x60
kasan_save_free_info+0x3b/0x60
__kasan_slab_free+0x5f/0x80
kfree+0x313/0x590
mgmt_pending_foreach+0x144/0x190
__mgmt_power_off+0xca/0x250
hci_dev_close_sync+0x8ba/0xb00
hci_set_powered_sync+0x384/0x480
hci_cmd_sync_work+0x187/0x210
process_one_work+0x4fd/0xbc0
worker_thread+0x2d8/0x570
kthread+0x1ad/0x1f0
ret_from_fork+0x3c9/0x540
ret_from_fork_asm+0x1a/0x30
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
net/bluetooth/mgmt.c | 37 ++++++++++++++++++-------------------
net/bluetooth/mgmt_util.c | 27 +++++++++++++++++++++++++++
net/bluetooth/mgmt_util.h | 2 ++
3 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d23ca1dd0893..91272640864a 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -960,19 +960,25 @@ static struct mgmt_pending_cmd *pending_find(u16 opcode, struct hci_dev *hdev)
return mgmt_pending_find(HCI_CHANNEL_CONTROL, opcode, hdev);
}
+static bool pending_find_copy(u16 opcode, struct hci_dev *hdev, void *data,
+ size_t len)
+{
+ return mgmt_pending_find_copy(HCI_CHANNEL_CONTROL, opcode, hdev,
+ data, len);
+}
+
u8 mgmt_get_adv_discov_flags(struct hci_dev *hdev)
{
- struct mgmt_pending_cmd *cmd;
+ struct mgmt_mode cp;
/* If there's a pending mgmt command the flags will not yet have
* their final values, so check for this first.
*/
- cmd = pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
- if (cmd) {
- struct mgmt_mode *cp = cmd->param;
- if (cp->val == 0x01)
+ if (pending_find_copy(MGMT_OP_SET_DISCOVERABLE, hdev, &cp,
+ sizeof(cp))) {
+ if (cp.val == 0x01)
return LE_AD_GENERAL;
- else if (cp->val == 0x02)
+ else if (cp.val == 0x02)
return LE_AD_LIMITED;
} else {
if (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE))
@@ -986,17 +992,13 @@ u8 mgmt_get_adv_discov_flags(struct hci_dev *hdev)
bool mgmt_get_connectable(struct hci_dev *hdev)
{
- struct mgmt_pending_cmd *cmd;
+ struct mgmt_mode cp;
/* If there's a pending mgmt command the flag will not yet have
* it's final value, so check for this first.
*/
- cmd = pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
- if (cmd) {
- struct mgmt_mode *cp = cmd->param;
-
- return cp->val;
- }
+ if (pending_find_copy(MGMT_OP_SET_CONNECTABLE, hdev, &cp, sizeof(cp)))
+ return cp.val;
return hci_dev_test_flag(hdev, HCI_CONNECTABLE);
}
@@ -9826,18 +9828,15 @@ static void unpair_device_rsp(struct mgmt_pending_cmd *cmd, void *data)
bool mgmt_powering_down(struct hci_dev *hdev)
{
- struct mgmt_pending_cmd *cmd;
- struct mgmt_mode *cp;
+ struct mgmt_mode cp;
if (hci_dev_test_flag(hdev, HCI_POWERING_DOWN))
return true;
- cmd = pending_find(MGMT_OP_SET_POWERED, hdev);
- if (!cmd)
+ if (!pending_find_copy(MGMT_OP_SET_POWERED, hdev, &cp, sizeof(cp)))
return false;
- cp = cmd->param;
- if (!cp->val)
+ if (!cp.val)
return true;
return false;
diff --git a/net/bluetooth/mgmt_util.c b/net/bluetooth/mgmt_util.c
index 6ea107c0e054..5d6d13ccadd2 100644
--- a/net/bluetooth/mgmt_util.c
+++ b/net/bluetooth/mgmt_util.c
@@ -233,6 +233,33 @@ struct mgmt_pending_cmd *mgmt_pending_find(unsigned short channel, u16 opcode,
return NULL;
}
+bool mgmt_pending_find_copy(unsigned short channel, u16 opcode,
+ struct hci_dev *hdev, void *data, size_t len)
+{
+ struct mgmt_pending_cmd *cmd, *tmp;
+ bool found = false;
+
+ mutex_lock(&hdev->mgmt_pending_lock);
+
+ list_for_each_entry_safe(cmd, tmp, &hdev->mgmt_pending, list) {
+ if (hci_sock_get_channel(cmd->sk) != channel)
+ continue;
+
+ if (cmd->opcode != opcode)
+ continue;
+
+ if (cmd->param_len >= len) {
+ memcpy(data, cmd->param, len);
+ found = true;
+ }
+ break;
+ }
+
+ mutex_unlock(&hdev->mgmt_pending_lock);
+
+ return found;
+}
+
void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev, bool remove,
void (*cb)(struct mgmt_pending_cmd *cmd, void *data),
void *data)
diff --git a/net/bluetooth/mgmt_util.h b/net/bluetooth/mgmt_util.h
index 20810cf06e81..4cccb71c8a1f 100644
--- a/net/bluetooth/mgmt_util.h
+++ b/net/bluetooth/mgmt_util.h
@@ -51,6 +51,8 @@ int mgmt_cmd_complete(struct sock *sk, u16 index, u16 cmd, u8 status,
struct mgmt_pending_cmd *mgmt_pending_find(unsigned short channel, u16 opcode,
struct hci_dev *hdev);
+bool mgmt_pending_find_copy(unsigned short channel, u16 opcode,
+ struct hci_dev *hdev, void *data, size_t len);
void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev, bool remove,
void (*cb)(struct mgmt_pending_cmd *cmd, void *data),
void *data);
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: copy pending command data under the list lock
2026-06-18 10:21 [PATCH] Bluetooth: mgmt: copy pending command data under the list lock Cen Zhang
@ 2026-06-18 15:46 ` Luiz Augusto von Dentz
2026-06-18 17:31 ` Cen Zhang
0 siblings, 1 reply; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2026-06-18 15:46 UTC (permalink / raw)
To: Cen Zhang; +Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, baijiaju1990
Hi Cen,
On Thu, Jun 18, 2026 at 6:21 AM Cen Zhang <zzzccc427@gmail.com> wrote:
>
> mgmt_pending_find() only protects the pending list lookup. Once it
> returns, a caller that dereferences the returned command has no lifetime
> guarantee unless another lock or ownership transfer keeps the command from
> being removed and freed.
>
> mgmt_powering_down() only needs the requested SET_POWERED mode, but it
> currently keeps the raw pending command pointer after the list lock has
> been dropped and then reads cmd->param and cp->val.
>
> The buggy scenario involves two paths, with each column showing the order
> within that path:
>
> hci_suspend_dev()/hci_resume_dev(): SET_POWERED completion:
> 1. mgmt_powering_down() calls 1. mgmt_set_powered_complete()
> pending_find()
> 2. pending_find() drops 2. mgmt_pending_valid() delists
> mgmt_pending_lock the command
> 3. mgmt_pending_free() frees the
> command and parameter buffer
> 3. mgmt_powering_down() reads
> cmd->param
> 4. mgmt_powering_down() reads
> cp->val
>
> Add a helper that copies a requested pending-command parameter slice while
> mgmt_pending_lock is still held. Use it for the mgmt_mode readers in
> mgmt.c so they take a snapshot of the pending request instead of keeping an
> unlocked raw command pointer just to inspect the requested mode.
>
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in mgmt_powering_down+0xa0/0xf0
>
> Call Trace:
> <TASK>
> dump_stack_lvl+0x66/0xa0
> print_report+0xce/0x5f0
> ? mgmt_powering_down+0xa0/0xf0
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __virt_addr_valid+0x19f/0x330
> ? mgmt_powering_down+0xa0/0xf0
> kasan_report+0xe0/0x110
> ? mgmt_powering_down+0xa0/0xf0
> mgmt_powering_down+0xa0/0xf0
> hci_suspend_dev+0xc0/0x2d0
> ? vhci_suspend_work+0x31/0x50
> process_one_work+0x4fd/0xbc0
> ? __pfx_process_one_work+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __list_add_valid_or_report+0x37/0xf0
> ? __pfx_vhci_suspend_work+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> worker_thread+0x2d8/0x570
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __pfx_worker_thread+0x10/0x10
> kthread+0x1ad/0x1f0
> ? __pfx_kthread+0x10/0x10
> ret_from_fork+0x3c9/0x540
> ? __pfx_ret_from_fork+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? __switch_to+0x2e9/0x730
> ? __pfx_kthread+0x10/0x10
> ret_from_fork_asm+0x1a/0x30
> </TASK>
>
> Allocated by task 336 on cpu 2 at 98.764916s:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x17/0x60
> __kasan_kmalloc+0xaa/0xb0
> mgmt_pending_new+0x44/0x130
> mgmt_pending_add+0x22/0x110
> set_powered+0x1ad/0x310
> hci_sock_sendmsg+0x96b/0xf80
> sock_write_iter+0x28e/0x2a0
> do_iter_readv_writev+0x211/0x390
> vfs_writev+0x266/0x7b0
> do_writev+0x191/0x1d0
> do_syscall_64+0x115/0x6a0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Freed by task 314 on cpu 0 at 101.816391s:
> kasan_save_stack+0x33/0x60
> kasan_save_track+0x17/0x60
> kasan_save_free_info+0x3b/0x60
> __kasan_slab_free+0x5f/0x80
> kfree+0x313/0x590
> mgmt_pending_foreach+0x144/0x190
> __mgmt_power_off+0xca/0x250
> hci_dev_close_sync+0x8ba/0xb00
> hci_set_powered_sync+0x384/0x480
> hci_cmd_sync_work+0x187/0x210
> process_one_work+0x4fd/0xbc0
> worker_thread+0x2d8/0x570
> kthread+0x1ad/0x1f0
> ret_from_fork+0x3c9/0x540
> ret_from_fork_asm+0x1a/0x30
>
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> net/bluetooth/mgmt.c | 37 ++++++++++++++++++-------------------
> net/bluetooth/mgmt_util.c | 27 +++++++++++++++++++++++++++
> net/bluetooth/mgmt_util.h | 2 ++
> 3 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index d23ca1dd0893..91272640864a 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -960,19 +960,25 @@ static struct mgmt_pending_cmd *pending_find(u16 opcode, struct hci_dev *hdev)
> return mgmt_pending_find(HCI_CHANNEL_CONTROL, opcode, hdev);
> }
>
> +static bool pending_find_copy(u16 opcode, struct hci_dev *hdev, void *data,
> + size_t len)
> +{
> + return mgmt_pending_find_copy(HCI_CHANNEL_CONTROL, opcode, hdev,
> + data, len);
> +}
> +
> u8 mgmt_get_adv_discov_flags(struct hci_dev *hdev)
> {
> - struct mgmt_pending_cmd *cmd;
> + struct mgmt_mode cp;
>
> /* If there's a pending mgmt command the flags will not yet have
> * their final values, so check for this first.
> */
> - cmd = pending_find(MGMT_OP_SET_DISCOVERABLE, hdev);
> - if (cmd) {
> - struct mgmt_mode *cp = cmd->param;
> - if (cp->val == 0x01)
> + if (pending_find_copy(MGMT_OP_SET_DISCOVERABLE, hdev, &cp,
> + sizeof(cp))) {
> + if (cp.val == 0x01)
> return LE_AD_GENERAL;
> - else if (cp->val == 0x02)
> + else if (cp.val == 0x02)
> return LE_AD_LIMITED;
> } else {
> if (hci_dev_test_flag(hdev, HCI_LIMITED_DISCOVERABLE))
> @@ -986,17 +992,13 @@ u8 mgmt_get_adv_discov_flags(struct hci_dev *hdev)
>
> bool mgmt_get_connectable(struct hci_dev *hdev)
> {
> - struct mgmt_pending_cmd *cmd;
> + struct mgmt_mode cp;
>
> /* If there's a pending mgmt command the flag will not yet have
> * it's final value, so check for this first.
> */
> - cmd = pending_find(MGMT_OP_SET_CONNECTABLE, hdev);
> - if (cmd) {
> - struct mgmt_mode *cp = cmd->param;
> -
> - return cp->val;
> - }
> + if (pending_find_copy(MGMT_OP_SET_CONNECTABLE, hdev, &cp, sizeof(cp)))
> + return cp.val;
>
> return hci_dev_test_flag(hdev, HCI_CONNECTABLE);
> }
> @@ -9826,18 +9828,15 @@ static void unpair_device_rsp(struct mgmt_pending_cmd *cmd, void *data)
>
> bool mgmt_powering_down(struct hci_dev *hdev)
> {
> - struct mgmt_pending_cmd *cmd;
> - struct mgmt_mode *cp;
> + struct mgmt_mode cp;
>
> if (hci_dev_test_flag(hdev, HCI_POWERING_DOWN))
> return true;
>
> - cmd = pending_find(MGMT_OP_SET_POWERED, hdev);
> - if (!cmd)
> + if (!pending_find_copy(MGMT_OP_SET_POWERED, hdev, &cp, sizeof(cp)))
> return false;
>
> - cp = cmd->param;
> - if (!cp->val)
> + if (!cp.val)
> return true;
>
> return false;
> diff --git a/net/bluetooth/mgmt_util.c b/net/bluetooth/mgmt_util.c
> index 6ea107c0e054..5d6d13ccadd2 100644
> --- a/net/bluetooth/mgmt_util.c
> +++ b/net/bluetooth/mgmt_util.c
> @@ -233,6 +233,33 @@ struct mgmt_pending_cmd *mgmt_pending_find(unsigned short channel, u16 opcode,
> return NULL;
> }
>
> +bool mgmt_pending_find_copy(unsigned short channel, u16 opcode,
> + struct hci_dev *hdev, void *data, size_t len)
> +{
> + struct mgmt_pending_cmd *cmd, *tmp;
> + bool found = false;
> +
> + mutex_lock(&hdev->mgmt_pending_lock);
> +
> + list_for_each_entry_safe(cmd, tmp, &hdev->mgmt_pending, list) {
> + if (hci_sock_get_channel(cmd->sk) != channel)
> + continue;
> +
> + if (cmd->opcode != opcode)
> + continue;
> +
> + if (cmd->param_len >= len) {
> + memcpy(data, cmd->param, len);
> + found = true;
> + }
> + break;
> + }
> +
> + mutex_unlock(&hdev->mgmt_pending_lock);
> +
> + return found;
> +}
Or we could just add a kref to mgmt_pending_cmd so mgmt_pending_find
can return a reference which is then unref'd when finished accessing
it.
> void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev, bool remove,
> void (*cb)(struct mgmt_pending_cmd *cmd, void *data),
> void *data)
> diff --git a/net/bluetooth/mgmt_util.h b/net/bluetooth/mgmt_util.h
> index 20810cf06e81..4cccb71c8a1f 100644
> --- a/net/bluetooth/mgmt_util.h
> +++ b/net/bluetooth/mgmt_util.h
> @@ -51,6 +51,8 @@ int mgmt_cmd_complete(struct sock *sk, u16 index, u16 cmd, u8 status,
>
> struct mgmt_pending_cmd *mgmt_pending_find(unsigned short channel, u16 opcode,
> struct hci_dev *hdev);
> +bool mgmt_pending_find_copy(unsigned short channel, u16 opcode,
> + struct hci_dev *hdev, void *data, size_t len);
> void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev, bool remove,
> void (*cb)(struct mgmt_pending_cmd *cmd, void *data),
> void *data);
> --
> 2.43.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: copy pending command data under the list lock
2026-06-18 15:46 ` Luiz Augusto von Dentz
@ 2026-06-18 17:31 ` Cen Zhang
0 siblings, 0 replies; 3+ messages in thread
From: Cen Zhang @ 2026-06-18 17:31 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, baijiaju1990
Hi Luiz,
> Or we could just add a kref to mgmt_pending_cmd so mgmt_pending_find
> can return a reference which is then unref'd when finished accessing
> it.
Thanks for the review and suggestion.
I looked into the kref approach. It does look cleaner, but it changes the
lifetime contract of mgmt_pending_find() and requires auditing all callers
and the removal/completion paths. I am not fully confident I can make that
broader mgmt_pending_cmd lifetime change correctly for this.
If you would prefer to make the kref-based change, I would really
appreciate it and would be happy to keep testing it with my reproducer.
If you would prefer me to respin it instead, I may need more specific
guidance.
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-18 17:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-18 10:21 [PATCH] Bluetooth: mgmt: copy pending command data under the list lock Cen Zhang
2026-06-18 15:46 ` Luiz Augusto von Dentz
2026-06-18 17:31 ` Cen Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®