Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update
@ 2026-07-03  6:00 Cen Zhang
  2026-07-03  6:41 ` bluez.test.bot
  2026-07-03 15:09 ` [PATCH] " Luiz Augusto von Dentz
  0 siblings, 2 replies; 4+ messages in thread
From: Cen Zhang @ 2026-07-03  6:00 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, baijiaju1990, zzzccc427

MGMT_OP_LOAD_CONN_PARAM queues conn_update_sync() when a single parameter
update changes an existing LE central connection. The queued work currently
stores the hci_conn_params object from hdev->le_conn_params. A later
LOAD_CONN_PARAM request can clear disabled parameters and free that object
before hci_cmd_sync_work() runs the queued callback.

The buggy scenario involves two paths, with each column showing the order
within that path:

  LOAD_CONN_PARAM update path:        LOAD_CONN_PARAM clear path:
  1. update the existing params       1. process a later multi-param load
  2. queue conn_update_sync(params)   2. clear disabled parameters
  3. return while the callback        3. free the queued params object
     is still pending
  4. callback dereferences params

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in conn_update_sync+0x2a/0xf0 [bluetooth]
Read of size 1 at addr ffff88810c697126 by task kworker/u17:0/377
Workqueue: hci0 hci_cmd_sync_work [bluetooth]

Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0xa0
 print_report+0xce/0x5f0
 ? conn_update_sync+0x2a/0xf0 [bluetooth]
 ? __virt_addr_valid+0x19f/0x330
 ? conn_update_sync+0x2a/0xf0 [bluetooth]
 kasan_report+0xe0/0x110
 ? conn_update_sync+0x2a/0xf0 [bluetooth]
 ? __pfx_conn_update_sync+0x10/0x10 [bluetooth]
 conn_update_sync+0x2a/0xf0 [bluetooth]
 hci_cmd_sync_work+0x187/0x210 [bluetooth]
 process_one_work+0x4fd/0xbc0
 worker_thread+0x2d8/0x570
 kthread+0x1ad/0x1f0
 ret_from_fork+0x3c9/0x540
 ret_from_fork_asm+0x1a/0x30

Allocated by task 466:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x17/0x60
 __kasan_kmalloc+0xaa/0xb0
 hci_conn_params_add+0xa6/0x240 [bluetooth]
 load_conn_param+0x4e1/0x850 [bluetooth]
 hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
 sock_write_iter+0x28e/0x2a0
 vfs_write+0x6e4/0x810
 ksys_write+0x147/0x170
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 474:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x17/0x60
 kasan_save_free_info+0x3b/0x60
 __kasan_slab_free+0x5f/0x80
 kfree+0x313/0x590
 hci_conn_params_clear_disabled+0x9b/0xc0 [bluetooth]
 load_conn_param+0x4bf/0x850 [bluetooth]
 hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
 sock_write_iter+0x28e/0x2a0
 vfs_write+0x6e4/0x810
 ksys_write+0x147/0x170
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Queue a private copy of the address and connection parameter values
instead, and free that copy from the sync-work destroy callback. The live
hdev->le_conn_params entry can then be cleared without invalidating queued
work.

Fixes: 0ece498c27d8c ("Bluetooth: MGMT: Make MGMT_OP_LOAD_CONN_PARAM update existing connection")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 net/bluetooth/mgmt.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 733a4b70e10c..4a38f7663445 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7947,6 +7947,11 @@ static int conn_update_sync(struct hci_dev *hdev, void *data)
 	return hci_le_conn_update_sync(hdev, conn, params);
 }
 
+static void conn_update_sync_destroy(struct hci_dev *hdev, void *data, int err)
+{
+	kfree(data);
+}
+
 static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
 			   u16 len)
 {
@@ -8054,9 +8059,28 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
 			    (conn->le_conn_min_interval != min ||
 			     conn->le_conn_max_interval != max ||
 			     conn->le_conn_latency != latency ||
-			     conn->le_supv_timeout != timeout))
-				hci_cmd_sync_queue(hdev, conn_update_sync,
-						   hci_param, NULL);
+			     conn->le_supv_timeout != timeout)) {
+				struct hci_conn_params *update;
+
+				/* Queue a private snapshot so the live params
+				 * entry can be freed later.
+				 */
+				update = kzalloc_obj(*update);
+				if (!update)
+					continue;
+
+				update->addr = hci_param->addr;
+				update->addr_type = hci_param->addr_type;
+				update->conn_min_interval = min;
+				update->conn_max_interval = max;
+				update->conn_latency = latency;
+				update->supervision_timeout = timeout;
+
+				if (hci_cmd_sync_queue(hdev, conn_update_sync,
+						       update,
+						       conn_update_sync_destroy) < 0)
+					kfree(update);
+			}
 		}
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update
  2026-07-03  6:00 [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update Cen Zhang
@ 2026-07-03  6:41 ` bluez.test.bot
  2026-07-03 15:09 ` [PATCH] " Luiz Augusto von Dentz
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2026-07-03  6:41 UTC (permalink / raw)
  To: linux-bluetooth, zzzccc427

[-- Attachment #1: Type: text/plain, Size: 1903 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/project/bluetooth/list/?series=1120896

---Test result---

Test Summary:
CheckPatch                    PASS      0.56 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.20 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      26.76 seconds
CheckAllWarning               PASS      29.20 seconds
CheckSparse                   PASS      27.84 seconds
BuildKernel32                 PASS      25.77 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      490.90 seconds
TestRunner_mgmt-tester        FAIL      205.90 seconds
TestRunner_mesh-tester        FAIL      24.85 seconds
IncrementalBuild              PASS      25.73 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.230 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.961 seconds
Mesh - Send cancel - 2                               Timed out    2.000 seconds


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

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update
  2026-07-03  6:00 [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update Cen Zhang
  2026-07-03  6:41 ` bluez.test.bot
@ 2026-07-03 15:09 ` Luiz Augusto von Dentz
  2026-07-03 16:54   ` Cen Zhang
  1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-07-03 15:09 UTC (permalink / raw)
  To: Cen Zhang; +Cc: Marcel Holtmann, linux-bluetooth, baijiaju1990

Hi Cen,

On Fri, Jul 3, 2026 at 2:00 AM Cen Zhang <zzzccc427@gmail.com> wrote:
>
> MGMT_OP_LOAD_CONN_PARAM queues conn_update_sync() when a single parameter
> update changes an existing LE central connection. The queued work currently
> stores the hci_conn_params object from hdev->le_conn_params. A later
> LOAD_CONN_PARAM request can clear disabled parameters and free that object
> before hci_cmd_sync_work() runs the queued callback.
>
> The buggy scenario involves two paths, with each column showing the order
> within that path:
>
>   LOAD_CONN_PARAM update path:        LOAD_CONN_PARAM clear path:
>   1. update the existing params       1. process a later multi-param load
>   2. queue conn_update_sync(params)   2. clear disabled parameters
>   3. return while the callback        3. free the queued params object
>      is still pending
>   4. callback dereferences params

This is valid though, which means the first update shouldn't even take
place if the parameters are freed while the callback is pending.

> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in conn_update_sync+0x2a/0xf0 [bluetooth]
> Read of size 1 at addr ffff88810c697126 by task kworker/u17:0/377
> Workqueue: hci0 hci_cmd_sync_work [bluetooth]
>
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x66/0xa0
>  print_report+0xce/0x5f0
>  ? conn_update_sync+0x2a/0xf0 [bluetooth]
>  ? __virt_addr_valid+0x19f/0x330
>  ? conn_update_sync+0x2a/0xf0 [bluetooth]
>  kasan_report+0xe0/0x110
>  ? conn_update_sync+0x2a/0xf0 [bluetooth]
>  ? __pfx_conn_update_sync+0x10/0x10 [bluetooth]
>  conn_update_sync+0x2a/0xf0 [bluetooth]
>  hci_cmd_sync_work+0x187/0x210 [bluetooth]
>  process_one_work+0x4fd/0xbc0
>  worker_thread+0x2d8/0x570
>  kthread+0x1ad/0x1f0
>  ret_from_fork+0x3c9/0x540
>  ret_from_fork_asm+0x1a/0x30
>
> Allocated by task 466:
>  kasan_save_stack+0x33/0x60
>  kasan_save_track+0x17/0x60
>  __kasan_kmalloc+0xaa/0xb0
>  hci_conn_params_add+0xa6/0x240 [bluetooth]
>  load_conn_param+0x4e1/0x850 [bluetooth]
>  hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
>  sock_write_iter+0x28e/0x2a0
>  vfs_write+0x6e4/0x810
>  ksys_write+0x147/0x170
>  do_syscall_64+0x115/0x6a0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Freed by task 474:
>  kasan_save_stack+0x33/0x60
>  kasan_save_track+0x17/0x60
>  kasan_save_free_info+0x3b/0x60
>  __kasan_slab_free+0x5f/0x80
>  kfree+0x313/0x590
>  hci_conn_params_clear_disabled+0x9b/0xc0 [bluetooth]
>  load_conn_param+0x4bf/0x850 [bluetooth]
>  hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
>  sock_write_iter+0x28e/0x2a0
>  vfs_write+0x6e4/0x810
>  ksys_write+0x147/0x170
>  do_syscall_64+0x115/0x6a0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> Queue a private copy of the address and connection parameter values
> instead, and free that copy from the sync-work destroy callback. The live
> hdev->le_conn_params entry can then be cleared without invalidating queued
> work.
>
> Fixes: 0ece498c27d8c ("Bluetooth: MGMT: Make MGMT_OP_LOAD_CONN_PARAM update existing connection")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  net/bluetooth/mgmt.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 733a4b70e10c..4a38f7663445 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -7947,6 +7947,11 @@ static int conn_update_sync(struct hci_dev *hdev, void *data)
>         return hci_le_conn_update_sync(hdev, conn, params);
>  }
>
> +static void conn_update_sync_destroy(struct hci_dev *hdev, void *data, int err)
> +{
> +       kfree(data);
> +}
> +
>  static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>                            u16 len)
>  {
> @@ -8054,9 +8059,28 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
>                             (conn->le_conn_min_interval != min ||
>                              conn->le_conn_max_interval != max ||
>                              conn->le_conn_latency != latency ||
> -                            conn->le_supv_timeout != timeout))
> -                               hci_cmd_sync_queue(hdev, conn_update_sync,
> -                                                  hci_param, NULL);
> +                            conn->le_supv_timeout != timeout)) {
> +                               struct hci_conn_params *update;
> +
> +                               /* Queue a private snapshot so the live params
> +                                * entry can be freed later.
> +                                */
> +                               update = kzalloc_obj(*update);
> +                               if (!update)
> +                                       continue;
> +
> +                               update->addr = hci_param->addr;
> +                               update->addr_type = hci_param->addr_type;
> +                               update->conn_min_interval = min;
> +                               update->conn_max_interval = max;
> +                               update->conn_latency = latency;
> +                               update->supervision_timeout = timeout;
> +
> +                               if (hci_cmd_sync_queue(hdev, conn_update_sync,
> +                                                      update,
> +                                                      conn_update_sync_destroy) < 0)
> +                                       kfree(update);
> +                       }

So this will probably need to do a lookup to check if hci_conn_params
have been removed then don't perform any update.

>                 }
>         }
>
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update
  2026-07-03 15:09 ` [PATCH] " Luiz Augusto von Dentz
@ 2026-07-03 16:54   ` Cen Zhang
  0 siblings, 0 replies; 4+ messages in thread
From: Cen Zhang @ 2026-07-03 16:54 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: Marcel Holtmann, linux-bluetooth, baijiaju1990

Hi Luiz,

Thanks for the review and suggestion. That makes sense.

I reworked this for v2 and I will send it soon.

Best regards,
Cen Zhang


Luiz Augusto von Dentz <luiz.dentz@gmail.com> 于2026年7月3日周五 23:10写道:
>
> Hi Cen,
>
> On Fri, Jul 3, 2026 at 2:00 AM Cen Zhang <zzzccc427@gmail.com> wrote:
> >
> > MGMT_OP_LOAD_CONN_PARAM queues conn_update_sync() when a single parameter
> > update changes an existing LE central connection. The queued work currently
> > stores the hci_conn_params object from hdev->le_conn_params. A later
> > LOAD_CONN_PARAM request can clear disabled parameters and free that object
> > before hci_cmd_sync_work() runs the queued callback.
> >
> > The buggy scenario involves two paths, with each column showing the order
> > within that path:
> >
> >   LOAD_CONN_PARAM update path:        LOAD_CONN_PARAM clear path:
> >   1. update the existing params       1. process a later multi-param load
> >   2. queue conn_update_sync(params)   2. clear disabled parameters
> >   3. return while the callback        3. free the queued params object
> >      is still pending
> >   4. callback dereferences params
>
> This is valid though, which means the first update shouldn't even take
> place if the parameters are freed while the callback is pending.
>
> > Validation reproduced this kernel report:
> > BUG: KASAN: slab-use-after-free in conn_update_sync+0x2a/0xf0 [bluetooth]
> > Read of size 1 at addr ffff88810c697126 by task kworker/u17:0/377
> > Workqueue: hci0 hci_cmd_sync_work [bluetooth]
> >
> > Call Trace:
> >  <TASK>
> >  dump_stack_lvl+0x66/0xa0
> >  print_report+0xce/0x5f0
> >  ? conn_update_sync+0x2a/0xf0 [bluetooth]
> >  ? __virt_addr_valid+0x19f/0x330
> >  ? conn_update_sync+0x2a/0xf0 [bluetooth]
> >  kasan_report+0xe0/0x110
> >  ? conn_update_sync+0x2a/0xf0 [bluetooth]
> >  ? __pfx_conn_update_sync+0x10/0x10 [bluetooth]
> >  conn_update_sync+0x2a/0xf0 [bluetooth]
> >  hci_cmd_sync_work+0x187/0x210 [bluetooth]
> >  process_one_work+0x4fd/0xbc0
> >  worker_thread+0x2d8/0x570
> >  kthread+0x1ad/0x1f0
> >  ret_from_fork+0x3c9/0x540
> >  ret_from_fork_asm+0x1a/0x30
> >
> > Allocated by task 466:
> >  kasan_save_stack+0x33/0x60
> >  kasan_save_track+0x17/0x60
> >  __kasan_kmalloc+0xaa/0xb0
> >  hci_conn_params_add+0xa6/0x240 [bluetooth]
> >  load_conn_param+0x4e1/0x850 [bluetooth]
> >  hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
> >  sock_write_iter+0x28e/0x2a0
> >  vfs_write+0x6e4/0x810
> >  ksys_write+0x147/0x170
> >  do_syscall_64+0x115/0x6a0
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > Freed by task 474:
> >  kasan_save_stack+0x33/0x60
> >  kasan_save_track+0x17/0x60
> >  kasan_save_free_info+0x3b/0x60
> >  __kasan_slab_free+0x5f/0x80
> >  kfree+0x313/0x590
> >  hci_conn_params_clear_disabled+0x9b/0xc0 [bluetooth]
> >  load_conn_param+0x4bf/0x850 [bluetooth]
> >  hci_sock_sendmsg+0x96b/0xf80 [bluetooth]
> >  sock_write_iter+0x28e/0x2a0
> >  vfs_write+0x6e4/0x810
> >  ksys_write+0x147/0x170
> >  do_syscall_64+0x115/0x6a0
> >  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > Queue a private copy of the address and connection parameter values
> > instead, and free that copy from the sync-work destroy callback. The live
> > hdev->le_conn_params entry can then be cleared without invalidating queued
> > work.
> >
> > Fixes: 0ece498c27d8c ("Bluetooth: MGMT: Make MGMT_OP_LOAD_CONN_PARAM update existing connection")
> > Assisted-by: Codex:gpt-5.5
> > Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> > ---
> >  net/bluetooth/mgmt.c | 30 +++++++++++++++++++++++++++---
> >  1 file changed, 27 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 733a4b70e10c..4a38f7663445 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -7947,6 +7947,11 @@ static int conn_update_sync(struct hci_dev *hdev, void *data)
> >         return hci_le_conn_update_sync(hdev, conn, params);
> >  }
> >
> > +static void conn_update_sync_destroy(struct hci_dev *hdev, void *data, int err)
> > +{
> > +       kfree(data);
> > +}
> > +
> >  static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> >                            u16 len)
> >  {
> > @@ -8054,9 +8059,28 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
> >                             (conn->le_conn_min_interval != min ||
> >                              conn->le_conn_max_interval != max ||
> >                              conn->le_conn_latency != latency ||
> > -                            conn->le_supv_timeout != timeout))
> > -                               hci_cmd_sync_queue(hdev, conn_update_sync,
> > -                                                  hci_param, NULL);
> > +                            conn->le_supv_timeout != timeout)) {
> > +                               struct hci_conn_params *update;
> > +
> > +                               /* Queue a private snapshot so the live params
> > +                                * entry can be freed later.
> > +                                */
> > +                               update = kzalloc_obj(*update);
> > +                               if (!update)
> > +                                       continue;
> > +
> > +                               update->addr = hci_param->addr;
> > +                               update->addr_type = hci_param->addr_type;
> > +                               update->conn_min_interval = min;
> > +                               update->conn_max_interval = max;
> > +                               update->conn_latency = latency;
> > +                               update->supervision_timeout = timeout;
> > +
> > +                               if (hci_cmd_sync_queue(hdev, conn_update_sync,
> > +                                                      update,
> > +                                                      conn_update_sync_destroy) < 0)
> > +                                       kfree(update);
> > +                       }
>
> So this will probably need to do a lookup to check if hci_conn_params
> have been removed then don't perform any update.
>
> >                 }
> >         }
> >
> > --
> > 2.43.0
> >
>
>
> --
> Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-03 16:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03  6:00 [PATCH] Bluetooth: MGMT: snapshot LOAD_CONN_PARAM data before queueing update Cen Zhang
2026-07-03  6:41 ` bluez.test.bot
2026-07-03 15:09 ` [PATCH] " Luiz Augusto von Dentz
2026-07-03 16:54   ` Cen Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox