* [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
@ 2025-10-03 19:07 Pauli Virtanen
2025-10-03 19:31 ` bluez.test.bot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Pauli Virtanen @ 2025-10-03 19:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen
There is a BUG: KASAN: stack-out-of-bounds in set_mesh_sync due to
memcpy from badly declared on-stack flexible array.
Another crash is in set_mesh_complete() due to double list_del via
mgmt_pending_valid + mgmt_pending_remove.
Use DEFINE_FLEX to declare the flexible array right, and don't memcpy
outside bounds.
As mgmt_pending_valid removes the cmd from list, use mgmt_pending_free,
and also report status on error.
Fixes: 302a1f674c00d ("Bluetooth: MGMT: Fix possible UAFs")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
include/net/bluetooth/mgmt.h | 2 +-
net/bluetooth/mgmt.c | 26 +++++++++++++++-----------
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 74edea06985b..bca0333f1e99 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -853,7 +853,7 @@ struct mgmt_cp_set_mesh {
__le16 window;
__le16 period;
__u8 num_ad_types;
- __u8 ad_types[];
+ __u8 ad_types[] __counted_by(num_ad_types);
} __packed;
#define MGMT_SET_MESH_RECEIVER_SIZE 6
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a3d16eece0d2..24e335e3a727 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2175,19 +2175,24 @@ static void set_mesh_complete(struct hci_dev *hdev, void *data, int err)
sk = cmd->sk;
if (status) {
+ mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER,
+ status);
mgmt_pending_foreach(MGMT_OP_SET_MESH_RECEIVER, hdev, true,
cmd_status_rsp, &status);
- return;
+ goto done;
}
- mgmt_pending_remove(cmd);
mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER, 0, NULL, 0);
+
+done:
+ mgmt_pending_free(cmd);
}
static int set_mesh_sync(struct hci_dev *hdev, void *data)
{
struct mgmt_pending_cmd *cmd = data;
- struct mgmt_cp_set_mesh cp;
+ DEFINE_FLEX(struct mgmt_cp_set_mesh, cp, ad_types, num_ad_types,
+ sizeof(hdev->mesh_ad_types));
size_t len;
mutex_lock(&hdev->mgmt_pending_lock);
@@ -2197,27 +2202,26 @@ static int set_mesh_sync(struct hci_dev *hdev, void *data)
return -ECANCELED;
}
- memcpy(&cp, cmd->param, sizeof(cp));
+ len = cmd->param_len;
+ memcpy(cp, cmd->param, min(__struct_size(cp), len));
mutex_unlock(&hdev->mgmt_pending_lock);
- len = cmd->param_len;
-
memset(hdev->mesh_ad_types, 0, sizeof(hdev->mesh_ad_types));
- if (cp.enable)
+ if (cp->enable)
hci_dev_set_flag(hdev, HCI_MESH);
else
hci_dev_clear_flag(hdev, HCI_MESH);
- hdev->le_scan_interval = __le16_to_cpu(cp.period);
- hdev->le_scan_window = __le16_to_cpu(cp.window);
+ hdev->le_scan_interval = __le16_to_cpu(cp->period);
+ hdev->le_scan_window = __le16_to_cpu(cp->window);
- len -= sizeof(cp);
+ len -= sizeof(struct mgmt_cp_set_mesh);
/* If filters don't fit, forward all adv pkts */
if (len <= sizeof(hdev->mesh_ad_types))
- memcpy(hdev->mesh_ad_types, cp.ad_types, len);
+ memcpy(hdev->mesh_ad_types, cp->ad_types, len);
hci_update_passive_scan_sync(hdev);
return 0;
--
2.51.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* RE: Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-03 19:07 [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete Pauli Virtanen
@ 2025-10-03 19:31 ` bluez.test.bot
2025-10-04 14:05 ` Pauli Virtanen
2025-10-04 15:46 ` [PATCH] " Paul Menzel
2025-10-06 15:20 ` patchwork-bot+bluetooth
2 siblings, 1 reply; 7+ messages in thread
From: bluez.test.bot @ 2025-10-03 19:31 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 2377 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=1008357
---Test result---
Test Summary:
CheckPatch PENDING 0.41 seconds
GitLint PENDING 0.33 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 24.57 seconds
CheckAllWarning PASS 27.18 seconds
CheckSparse PASS 30.39 seconds
BuildKernel32 PASS 24.51 seconds
TestRunnerSetup PASS 486.03 seconds
TestRunner_l2cap-tester PASS 24.06 seconds
TestRunner_iso-tester PASS 57.49 seconds
TestRunner_bnep-tester PASS 6.16 seconds
TestRunner_mgmt-tester FAIL 125.14 seconds
TestRunner_rfcomm-tester PASS 9.23 seconds
TestRunner_sco-tester PASS 14.38 seconds
TestRunner_ioctl-tester PASS 10.19 seconds
TestRunner_mesh-tester FAIL 11.31 seconds
TestRunner_smp-tester PASS 8.46 seconds
TestRunner_userchan-tester PASS 6.41 seconds
IncrementalBuild PENDING 0.78 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4
Failed Test Cases
Read Exp Feature - Success Failed 0.108 seconds
LL Privacy - Set Device Flag 1 (Device Privacy) Failed 0.162 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.886 seconds
Mesh - Send cancel - 2 Timed out 1.996 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-03 19:31 ` bluez.test.bot
@ 2025-10-04 14:05 ` Pauli Virtanen
0 siblings, 0 replies; 7+ messages in thread
From: Pauli Virtanen @ 2025-10-04 14:05 UTC (permalink / raw)
To: linux-bluetooth
pe, 2025-10-03 kello 12:31 -0700, bluez.test.bot@gmail.com kirjoitti:
> 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=1008357
>
> ---Test result---
>
> Test Summary:
> CheckPatch PENDING 0.41 seconds
> GitLint PENDING 0.33 seconds
> SubjectPrefix PASS 0.10 seconds
> BuildKernel PASS 24.57 seconds
> CheckAllWarning PASS 27.18 seconds
> CheckSparse PASS 30.39 seconds
> BuildKernel32 PASS 24.51 seconds
> TestRunnerSetup PASS 486.03 seconds
> TestRunner_l2cap-tester PASS 24.06 seconds
> TestRunner_iso-tester PASS 57.49 seconds
> TestRunner_bnep-tester PASS 6.16 seconds
> TestRunner_mgmt-tester FAIL 125.14 seconds
> TestRunner_rfcomm-tester PASS 9.23 seconds
> TestRunner_sco-tester PASS 14.38 seconds
> TestRunner_ioctl-tester PASS 10.19 seconds
> TestRunner_mesh-tester FAIL 11.31 seconds
> TestRunner_smp-tester PASS 8.46 seconds
> TestRunner_userchan-tester PASS 6.41 seconds
> IncrementalBuild PENDING 0.78 seconds
>
> Details
> ##############################
> Test: CheckPatch - PENDING
> Desc: Run checkpatch.pl script
> Output:
>
> ##############################
> Test: GitLint - PENDING
> Desc: Run gitlint
> Output:
>
> ##############################
> Test: TestRunner_mgmt-tester - FAIL
> Desc: Run mgmt-tester with test-runner
> Output:
> Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4
>
> Failed Test Cases
> Read Exp Feature - Success Failed 0.108 seconds
> LL Privacy - Set Device Flag 1 (Device Privacy) Failed 0.162 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.886 seconds
> Mesh - Send cancel - 2 Timed out 1.996 seconds
These mesh-tester failures appear to be older, see eg.
https://lore.kernel.org/linux-bluetooth/68c66f83.050a0220.1b1f43.486b@mx.google.com/
Probably some other bug than this.
The reason why the test bot is currently not sending out email reports
for several of the patches seems to be because the bug fixed in this
patch causes mesh-tester to hang the kernel, and the bot worker times
out. See eg
https://github.com/BluezTestBot/bluetooth-next/actions/runs/18228972296/job/51907378892?pr=3102
2025-10-03 17:32:05,593:DEBUG :> Oops: general protection fault, probably for non-canonical address 0xfbd59c0000000021: 0000 [#1] KASAN NOPTI
2025-10-03 17:32:05,594:DEBUG :> KASAN: maybe wild-memory-access in range [0xdead000000000108-0xdead00000000010f]
2025-10-03 17:32:05,596:DEBUG :> CPU: 0 UID: 0 PID: 45 Comm: kworker/u5:0 Not tainted 6.17.0-rc7-gd82d3cf940db #1 PREEMPT(none)
2025-10-03 17:32:05,596:DEBUG :> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
2025-10-03 17:32:05,598:DEBUG :> Workqueue: hci0 hci_cmd_sync_work
2025-10-03 17:32:05,598:DEBUG :> RIP: 0010:mgmt_pending_remove+0x98/0x1b0
2025-10-03 17:32:05,599:DEBUG :> Code: 89 f2 48 c1 ea 03 80 3c 02 00 0f 85 11 01 00 00 49 8d 7c 24 08 48 8b 5d 08 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 e3 00 00 00 48 89 da 49 89 5c 24 08 48 b8 00 00
2025-10-03 17:32:05,600:DEBUG :> RSP: 0018:ffff888001d97a88 EFLAGS: 00000206
2025-10-03 17:32:05,600:DEBUG :> RAX: dffffc0000000000 RBX: dead000000000122 RCX: ffffffff94e750e3
2025-10-03 17:32:05,600:DEBUG :> RDX: 1bd5a00000000021 RSI: 0000000000000008 RDI: dead000000000108
2025-10-03 17:32:05,601:DEBUG :> RBP: ffff88800245b000 R08: 0000000000000000 R09: ffffed10003b2f33
2025-10-03 17:32:05,601:DEBUG :> R10: ffff888001d97a88 R11: ffff888001d97997 R12: dead000000000100
2025-10-03 17:32:05,601:DEBUG :> R13: ffff88800245b018 R14: ffff88800245b008 R15: ffff88800218b200
2025-10-03 17:32:05,602:DEBUG :> FS: 0000000000000000(0000) GS:0000000000000000(0000) knlGS:0000000000000000
2025-10-03 17:32:05,602:DEBUG :> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
2025-10-03 17:32:05,602:DEBUG :> CR2: 00007ffde0d53ff8 CR3: 0000000002598000 CR4: 00000000000006f0
2025-10-03 17:32:05,602:DEBUG :> Call Trace:
2025-10-03 17:32:05,603:DEBUG :> <TASK>
2025-10-03 17:32:05,603:DEBUG :> set_mesh_complete+0xc5/0x1d0
2025-10-03 17:32:05,603:DEBUG :> ? __pfx_set_mesh_complete+0x10/0x10
2025-10-03 17:32:05,603:DEBUG :> hci_cmd_sync_work+0x243/0x3c0
> ##############################
> Test: IncrementalBuild - PENDING
> Desc: Incremental build with the patches in the series
> Output:
>
>
>
> ---
> Regards,
> Linux Bluetooth
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-03 19:07 [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete Pauli Virtanen
2025-10-03 19:31 ` bluez.test.bot
@ 2025-10-04 15:46 ` Paul Menzel
2025-10-04 16:04 ` Pauli Virtanen
2025-10-06 15:20 ` patchwork-bot+bluetooth
2 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2025-10-04 15:46 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Dear Pauli,
Thank you for your patch.
Am 03.10.25 um 21:07 schrieb Pauli Virtanen:
> There is a BUG: KASAN: stack-out-of-bounds in set_mesh_sync due to
> memcpy from badly declared on-stack flexible array.
>
> Another crash is in set_mesh_complete() due to double list_del via
> mgmt_pending_valid + mgmt_pending_remove.
>
> Use DEFINE_FLEX to declare the flexible array right, and don't memcpy
> outside bounds.
>
> As mgmt_pending_valid removes the cmd from list, use mgmt_pending_free,
> and also report status on error.
>
> Fixes: 302a1f674c00d ("Bluetooth: MGMT: Fix possible UAFs")
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
> include/net/bluetooth/mgmt.h | 2 +-
> net/bluetooth/mgmt.c | 26 +++++++++++++++-----------
> 2 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 74edea06985b..bca0333f1e99 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -853,7 +853,7 @@ struct mgmt_cp_set_mesh {
> __le16 window;
> __le16 period;
> __u8 num_ad_types;
> - __u8 ad_types[];
> + __u8 ad_types[] __counted_by(num_ad_types);
> } __packed;
> #define MGMT_SET_MESH_RECEIVER_SIZE 6
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index a3d16eece0d2..24e335e3a727 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -2175,19 +2175,24 @@ static void set_mesh_complete(struct hci_dev *hdev, void *data, int err)
> sk = cmd->sk;
>
> if (status) {
> + mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER,
> + status);
> mgmt_pending_foreach(MGMT_OP_SET_MESH_RECEIVER, hdev, true,
> cmd_status_rsp, &status);
> - return;
> + goto done;
> }
>
> - mgmt_pending_remove(cmd);
> mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER, 0, NULL, 0);
> +
> +done:
> + mgmt_pending_free(cmd);
> }
>
> static int set_mesh_sync(struct hci_dev *hdev, void *data)
> {
> struct mgmt_pending_cmd *cmd = data;
> - struct mgmt_cp_set_mesh cp;
> + DEFINE_FLEX(struct mgmt_cp_set_mesh, cp, ad_types, num_ad_types,
> + sizeof(hdev->mesh_ad_types));
> size_t len;
>
> mutex_lock(&hdev->mgmt_pending_lock);
> @@ -2197,27 +2202,26 @@ static int set_mesh_sync(struct hci_dev *hdev, void *data)
> return -ECANCELED;
> }
>
> - memcpy(&cp, cmd->param, sizeof(cp));
> + len = cmd->param_len;
> + memcpy(cp, cmd->param, min(__struct_size(cp), len));
>
> mutex_unlock(&hdev->mgmt_pending_lock);
>
> - len = cmd->param_len;
> -
> memset(hdev->mesh_ad_types, 0, sizeof(hdev->mesh_ad_types));
>
> - if (cp.enable)
> + if (cp->enable)
> hci_dev_set_flag(hdev, HCI_MESH);
> else
> hci_dev_clear_flag(hdev, HCI_MESH);
>
> - hdev->le_scan_interval = __le16_to_cpu(cp.period);
> - hdev->le_scan_window = __le16_to_cpu(cp.window);
> + hdev->le_scan_interval = __le16_to_cpu(cp->period);
> + hdev->le_scan_window = __le16_to_cpu(cp->window);
>
> - len -= sizeof(cp);
> + len -= sizeof(struct mgmt_cp_set_mesh);
>
> /* If filters don't fit, forward all adv pkts */
> if (len <= sizeof(hdev->mesh_ad_types))
> - memcpy(hdev->mesh_ad_types, cp.ad_types, len);
> + memcpy(hdev->mesh_ad_types, cp->ad_types, len);
>
> hci_update_passive_scan_sync(hdev);
> return 0;
Would it be possible to make this two commits? Either way:
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-04 15:46 ` [PATCH] " Paul Menzel
@ 2025-10-04 16:04 ` Pauli Virtanen
2025-10-04 16:11 ` Paul Menzel
0 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2025-10-04 16:04 UTC (permalink / raw)
To: Paul Menzel; +Cc: linux-bluetooth
la, 2025-10-04 kello 17:46 +0200, Paul Menzel kirjoitti:
> Dear Pauli,
>
>
> Thank you for your patch.
>
>
> Am 03.10.25 um 21:07 schrieb Pauli Virtanen:
> > There is a BUG: KASAN: stack-out-of-bounds in set_mesh_sync due to
> > memcpy from badly declared on-stack flexible array.
> >
> > Another crash is in set_mesh_complete() due to double list_del via
> > mgmt_pending_valid + mgmt_pending_remove.
> >
> > Use DEFINE_FLEX to declare the flexible array right, and don't memcpy
> > outside bounds.
> >
> > As mgmt_pending_valid removes the cmd from list, use mgmt_pending_free,
> > and also report status on error.
> >
> > Fixes: 302a1f674c00d ("Bluetooth: MGMT: Fix possible UAFs")
> > Signed-off-by: Pauli Virtanen <pav@iki.fi>
> > ---
> > include/net/bluetooth/mgmt.h | 2 +-
> > net/bluetooth/mgmt.c | 26 +++++++++++++++-----------
> > 2 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> > index 74edea06985b..bca0333f1e99 100644
> > --- a/include/net/bluetooth/mgmt.h
> > +++ b/include/net/bluetooth/mgmt.h
> > @@ -853,7 +853,7 @@ struct mgmt_cp_set_mesh {
> > __le16 window;
> > __le16 period;
> > __u8 num_ad_types;
> > - __u8 ad_types[];
> > + __u8 ad_types[] __counted_by(num_ad_types);
> > } __packed;
> > #define MGMT_SET_MESH_RECEIVER_SIZE 6
> >
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index a3d16eece0d2..24e335e3a727 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -2175,19 +2175,24 @@ static void set_mesh_complete(struct hci_dev *hdev, void *data, int err)
> > sk = cmd->sk;
> >
> > if (status) {
> > + mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER,
> > + status);
> > mgmt_pending_foreach(MGMT_OP_SET_MESH_RECEIVER, hdev, true,
> > cmd_status_rsp, &status);
> > - return;
> > + goto done;
> > }
> >
> > - mgmt_pending_remove(cmd);
> > mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER, 0, NULL, 0);
> > +
> > +done:
> > + mgmt_pending_free(cmd);
> > }
> >
> > static int set_mesh_sync(struct hci_dev *hdev, void *data)
> > {
> > struct mgmt_pending_cmd *cmd = data;
> > - struct mgmt_cp_set_mesh cp;
> > + DEFINE_FLEX(struct mgmt_cp_set_mesh, cp, ad_types, num_ad_types,
> > + sizeof(hdev->mesh_ad_types));
> > size_t len;
> >
> > mutex_lock(&hdev->mgmt_pending_lock);
> > @@ -2197,27 +2202,26 @@ static int set_mesh_sync(struct hci_dev *hdev, void *data)
> > return -ECANCELED;
> > }
> >
> > - memcpy(&cp, cmd->param, sizeof(cp));
> > + len = cmd->param_len;
> > + memcpy(cp, cmd->param, min(__struct_size(cp), len));
> >
> > mutex_unlock(&hdev->mgmt_pending_lock);
> >
> > - len = cmd->param_len;
> > -
> > memset(hdev->mesh_ad_types, 0, sizeof(hdev->mesh_ad_types));
> >
> > - if (cp.enable)
> > + if (cp->enable)
> > hci_dev_set_flag(hdev, HCI_MESH);
> > else
> > hci_dev_clear_flag(hdev, HCI_MESH);
> >
> > - hdev->le_scan_interval = __le16_to_cpu(cp.period);
> > - hdev->le_scan_window = __le16_to_cpu(cp.window);
> > + hdev->le_scan_interval = __le16_to_cpu(cp->period);
> > + hdev->le_scan_window = __le16_to_cpu(cp->window);
> >
> > - len -= sizeof(cp);
> > + len -= sizeof(struct mgmt_cp_set_mesh);
> >
> > /* If filters don't fit, forward all adv pkts */
> > if (len <= sizeof(hdev->mesh_ad_types))
> > - memcpy(hdev->mesh_ad_types, cp.ad_types, len);
> > + memcpy(hdev->mesh_ad_types, cp->ad_types, len);
> >
> > hci_update_passive_scan_sync(hdev);
> > return 0;
>
> Would it be possible to make this two commits? Either way:
>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Both issues were introduced in 302a1f674c00d, which is why also the fix
here is a single patch.
>
>
> Kind regards,
>
> Paul
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-04 16:04 ` Pauli Virtanen
@ 2025-10-04 16:11 ` Paul Menzel
0 siblings, 0 replies; 7+ messages in thread
From: Paul Menzel @ 2025-10-04 16:11 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Dear Pauli,
Am 04.10.25 um 18:04 schrieb Pauli Virtanen:
> la, 2025-10-04 kello 17:46 +0200, Paul Menzel kirjoitti:
>> Am 03.10.25 um 21:07 schrieb Pauli Virtanen:
>>> There is a BUG: KASAN: stack-out-of-bounds in set_mesh_sync due to
>>> memcpy from badly declared on-stack flexible array.
>>>
>>> Another crash is in set_mesh_complete() due to double list_del via
>>> mgmt_pending_valid + mgmt_pending_remove.
>>>
>>> Use DEFINE_FLEX to declare the flexible array right, and don't memcpy
>>> outside bounds.
>>>
>>> As mgmt_pending_valid removes the cmd from list, use mgmt_pending_free,
>>> and also report status on error.
>>>
>>> Fixes: 302a1f674c00d ("Bluetooth: MGMT: Fix possible UAFs")
>>> Signed-off-by: Pauli Virtanen <pav@iki.fi>
>>> ---
>>> include/net/bluetooth/mgmt.h | 2 +-
>>> net/bluetooth/mgmt.c | 26 +++++++++++++++-----------
>>> 2 files changed, 16 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>>> index 74edea06985b..bca0333f1e99 100644
>>> --- a/include/net/bluetooth/mgmt.h
>>> +++ b/include/net/bluetooth/mgmt.h
>>> @@ -853,7 +853,7 @@ struct mgmt_cp_set_mesh {
>>> __le16 window;
>>> __le16 period;
>>> __u8 num_ad_types;
>>> - __u8 ad_types[];
>>> + __u8 ad_types[] __counted_by(num_ad_types);
>>> } __packed;
>>> #define MGMT_SET_MESH_RECEIVER_SIZE 6
>>>
>>> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
>>> index a3d16eece0d2..24e335e3a727 100644
>>> --- a/net/bluetooth/mgmt.c
>>> +++ b/net/bluetooth/mgmt.c
>>> @@ -2175,19 +2175,24 @@ static void set_mesh_complete(struct hci_dev *hdev, void *data, int err)
>>> sk = cmd->sk;
>>>
>>> if (status) {
>>> + mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER,
>>> + status);
>>> mgmt_pending_foreach(MGMT_OP_SET_MESH_RECEIVER, hdev, true,
>>> cmd_status_rsp, &status);
>>> - return;
>>> + goto done;
>>> }
>>>
>>> - mgmt_pending_remove(cmd);
>>> mgmt_cmd_complete(sk, hdev->id, MGMT_OP_SET_MESH_RECEIVER, 0, NULL, 0);
>>> +
>>> +done:
>>> + mgmt_pending_free(cmd);
>>> }
>>>
>>> static int set_mesh_sync(struct hci_dev *hdev, void *data)
>>> {
>>> struct mgmt_pending_cmd *cmd = data;
>>> - struct mgmt_cp_set_mesh cp;
>>> + DEFINE_FLEX(struct mgmt_cp_set_mesh, cp, ad_types, num_ad_types,
>>> + sizeof(hdev->mesh_ad_types));
>>> size_t len;
>>>
>>> mutex_lock(&hdev->mgmt_pending_lock);
>>> @@ -2197,27 +2202,26 @@ static int set_mesh_sync(struct hci_dev *hdev, void *data)
>>> return -ECANCELED;
>>> }
>>>
>>> - memcpy(&cp, cmd->param, sizeof(cp));
>>> + len = cmd->param_len;
>>> + memcpy(cp, cmd->param, min(__struct_size(cp), len));
>>>
>>> mutex_unlock(&hdev->mgmt_pending_lock);
>>>
>>> - len = cmd->param_len;
>>> -
>>> memset(hdev->mesh_ad_types, 0, sizeof(hdev->mesh_ad_types));
>>>
>>> - if (cp.enable)
>>> + if (cp->enable)
>>> hci_dev_set_flag(hdev, HCI_MESH);
>>> else
>>> hci_dev_clear_flag(hdev, HCI_MESH);
>>>
>>> - hdev->le_scan_interval = __le16_to_cpu(cp.period);
>>> - hdev->le_scan_window = __le16_to_cpu(cp.window);
>>> + hdev->le_scan_interval = __le16_to_cpu(cp->period);
>>> + hdev->le_scan_window = __le16_to_cpu(cp->window);
>>>
>>> - len -= sizeof(cp);
>>> + len -= sizeof(struct mgmt_cp_set_mesh);
>>>
>>> /* If filters don't fit, forward all adv pkts */
>>> if (len <= sizeof(hdev->mesh_ad_types))
>>> - memcpy(hdev->mesh_ad_types, cp.ad_types, len);
>>> + memcpy(hdev->mesh_ad_types, cp->ad_types, len);
>>>
>>> hci_update_passive_scan_sync(hdev);
>>> return 0;
>>
>> Would it be possible to make this two commits? Either way:
>>
>> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
> Both issues were introduced in 302a1f674c00d, which is why also the fix
> here is a single patch.
Hmm, for me this is not a strong reason. In my humble opinion, two
commits would make it patch smaller and easier to review, and would
improve bisect/pinpoint results and allow easier reverts.
Kind regards,
Paul
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
2025-10-03 19:07 [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete Pauli Virtanen
2025-10-03 19:31 ` bluez.test.bot
2025-10-04 15:46 ` [PATCH] " Paul Menzel
@ 2025-10-06 15:20 ` patchwork-bot+bluetooth
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2025-10-06 15:20 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
On Fri, 3 Oct 2025 22:07:32 +0300 you wrote:
> There is a BUG: KASAN: stack-out-of-bounds in set_mesh_sync due to
> memcpy from badly declared on-stack flexible array.
>
> Another crash is in set_mesh_complete() due to double list_del via
> mgmt_pending_valid + mgmt_pending_remove.
>
> Use DEFINE_FLEX to declare the flexible array right, and don't memcpy
> outside bounds.
>
> [...]
Here is the summary with links:
- Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete
https://git.kernel.org/bluetooth/bluetooth-next/c/fea4150ae678
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-06 15:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 19:07 [PATCH] Bluetooth: MGMT: fix crash in set_mesh_sync and set_mesh_complete Pauli Virtanen
2025-10-03 19:31 ` bluez.test.bot
2025-10-04 14:05 ` Pauli Virtanen
2025-10-04 15:46 ` [PATCH] " Paul Menzel
2025-10-04 16:04 ` Pauli Virtanen
2025-10-04 16:11 ` Paul Menzel
2025-10-06 15:20 ` patchwork-bot+bluetooth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox