Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()
@ 2026-06-01 16:22 Xiang Mei
  2026-06-01 17:38 ` Luiz Augusto von Dentz
  2026-06-01 18:59 ` bluez.test.bot
  0 siblings, 2 replies; 4+ messages in thread
From: Xiang Mei @ 2026-06-01 16:22 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: Marcel Holtmann, Luiz Augusto von Dentz, Weiming Shi,
	linux-kernel, Xiang Mei

From: Weiming Shi <bestswngs@gmail.com>

eir_create_adv_data() builds the advertising data into a fixed-size
buffer of "size" bytes (31 for the legacy path in hci_set_adv_data_sync()).
It may prepend a 3-byte "Flags" AD structure, but the per-instance
advertising data is then copied without checking that it still fits:

skip_flags:
	if (adv) {
		memcpy(ptr, adv->adv_data, adv->adv_data_len);

The "Flags" structure is added whenever BR/EDR is disabled
(LE_AD_NO_BREDR), which is the normal state for an LE-only controller.
However, the mgmt length validator tlv_data_max_len() only reserves
those 3 bytes when the user-supplied adv_flags carries a managed-flags
bit (DISCOV / LIMITED_DISCOV / MANAGED_FLAGS). Adding an instance with
flags == 0 reserves nothing, so adv_data_len is accepted up to the full
buffer size. At advertise time the 3 flag bytes are still prepended,
and the subsequent memcpy() writes 3 + adv_data_len bytes into the
size-byte buffer, overflowing it by the attacker-controlled tail of
adv_data:

  BUG: KASAN: stack-out-of-bounds in eir_create_adv_data (net/bluetooth/eir.c:302)
  Write of size 31 at addr ffff88800b7e7bac by task kworker/u9:0/51
  Workqueue: hci0 hci_cmd_sync_work
   __asan_memcpy (mm/kasan/shadow.c:106)
   eir_create_adv_data (net/bluetooth/eir.c:302)
   hci_update_adv_data_sync (net/bluetooth/hci_sync.c:1689)
   hci_schedule_adv_instance_sync (net/bluetooth/hci_sync.c:2015)
   hci_cmd_sync_work (net/bluetooth/hci_sync.c:332)
  This frame has 1 object:
   [32, 64) 'cp'

The inconsistency dates back to when the managed "Flags" field was first
added to the Add Advertising path: the prepended LE_AD_NO_BREDR flag does
not depend on the user-supplied adv_flags, but tlv_data_is_valid() only
reserved room when MGMT_ADV_FLAG_DISCOV was set. Commit 47c03902269a
("Bluetooth: eir: Fix possible crashes on eir_create_adv_data") later
added the "size" argument and bounds-checked the "Flags" and "Tx Power"
AD structures, but left this copy unguarded. Guard it the same way so
the data is only copied when it fits.

The bug is reachable by a local user with CAP_NET_ADMIN that owns an
LE-only controller using the legacy advertising path.

Fixes: b44133ff03be ("Bluetooth: Support the "discoverable" adv flag")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/bluetooth/eir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/eir.c b/net/bluetooth/eir.c
index 3f72111ba651..e574f8f61e16 100644
--- a/net/bluetooth/eir.c
+++ b/net/bluetooth/eir.c
@@ -297,7 +297,7 @@ u8 eir_create_adv_data(struct hci_dev *hdev, u8 instance, u8 *ptr, u8 size)
 	}
 
 skip_flags:
-	if (adv) {
+	if (adv && ad_len + adv->adv_data_len <= size) {
 		memcpy(ptr, adv->adv_data, adv->adv_data_len);
 		ad_len += adv->adv_data_len;
 		ptr += adv->adv_data_len;
-- 
2.43.0


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

* Re: [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()
  2026-06-01 16:22 [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data() Xiang Mei
@ 2026-06-01 17:38 ` Luiz Augusto von Dentz
  2026-06-02 16:14   ` Weiming Shi
  2026-06-01 18:59 ` bluez.test.bot
  1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-06-01 17:38 UTC (permalink / raw)
  To: Xiang Mei; +Cc: linux-bluetooth, Marcel Holtmann, linux-kernel, Xiang Mei

Hi Xiang,

On Mon, Jun 1, 2026 at 12:22 PM Xiang Mei <bestswngs@gmail.com> wrote:
>
> From: Weiming Shi <bestswngs@gmail.com>
>
> eir_create_adv_data() builds the advertising data into a fixed-size
> buffer of "size" bytes (31 for the legacy path in hci_set_adv_data_sync()).
> It may prepend a 3-byte "Flags" AD structure, but the per-instance
> advertising data is then copied without checking that it still fits:
>
> skip_flags:
>         if (adv) {
>                 memcpy(ptr, adv->adv_data, adv->adv_data_len);
>
> The "Flags" structure is added whenever BR/EDR is disabled
> (LE_AD_NO_BREDR), which is the normal state for an LE-only controller.
> However, the mgmt length validator tlv_data_max_len() only reserves
> those 3 bytes when the user-supplied adv_flags carries a managed-flags
> bit (DISCOV / LIMITED_DISCOV / MANAGED_FLAGS). Adding an instance with
> flags == 0 reserves nothing, so adv_data_len is accepted up to the full
> buffer size. At advertise time the 3 flag bytes are still prepended,
> and the subsequent memcpy() writes 3 + adv_data_len bytes into the
> size-byte buffer, overflowing it by the attacker-controlled tail of
> adv_data:
>
>   BUG: KASAN: stack-out-of-bounds in eir_create_adv_data (net/bluetooth/eir.c:302)
>   Write of size 31 at addr ffff88800b7e7bac by task kworker/u9:0/51
>   Workqueue: hci0 hci_cmd_sync_work
>    __asan_memcpy (mm/kasan/shadow.c:106)
>    eir_create_adv_data (net/bluetooth/eir.c:302)
>    hci_update_adv_data_sync (net/bluetooth/hci_sync.c:1689)
>    hci_schedule_adv_instance_sync (net/bluetooth/hci_sync.c:2015)
>    hci_cmd_sync_work (net/bluetooth/hci_sync.c:332)
>   This frame has 1 object:
>    [32, 64) 'cp'

Add the btmon trace of the MGMT command when this is triggered, and
explaing how the advertisement was created, as with bluetoothd?

> The inconsistency dates back to when the managed "Flags" field was first
> added to the Add Advertising path: the prepended LE_AD_NO_BREDR flag does
> not depend on the user-supplied adv_flags, but tlv_data_is_valid() only
> reserved room when MGMT_ADV_FLAG_DISCOV was set. Commit 47c03902269a
> ("Bluetooth: eir: Fix possible crashes on eir_create_adv_data") later
> added the "size" argument and bounds-checked the "Flags" and "Tx Power"
> AD structures, but left this copy unguarded. Guard it the same way so
> the data is only copied when it fits.
>
> The bug is reachable by a local user with CAP_NET_ADMIN that owns an
> LE-only controller using the legacy advertising path.
>
> Fixes: b44133ff03be ("Bluetooth: Support the "discoverable" adv flag")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  net/bluetooth/eir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/eir.c b/net/bluetooth/eir.c
> index 3f72111ba651..e574f8f61e16 100644
> --- a/net/bluetooth/eir.c
> +++ b/net/bluetooth/eir.c
> @@ -297,7 +297,7 @@ u8 eir_create_adv_data(struct hci_dev *hdev, u8 instance, u8 *ptr, u8 size)
>         }
>
>  skip_flags:
> -       if (adv) {
> +       if (adv && ad_len + adv->adv_data_len <= size) {

So we have 2 options: 1) Don't add flags if there is no space, or 2)
Don't add the user provided data. We should probably choose option 1,
not option 2 since option 2 probably means the advertisement is
useless.

>                 memcpy(ptr, adv->adv_data, adv->adv_data_len);
>                 ad_len += adv->adv_data_len;
>                 ptr += adv->adv_data_len;
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz

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

* RE: Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()
  2026-06-01 16:22 [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data() Xiang Mei
  2026-06-01 17:38 ` Luiz Augusto von Dentz
@ 2026-06-01 18:59 ` bluez.test.bot
  1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2026-06-01 18:59 UTC (permalink / raw)
  To: linux-bluetooth, bestswngs

[-- Attachment #1: Type: text/plain, Size: 3408 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=1104138

---Test result---

Test Summary:
CheckPatch                    FAIL      0.78 seconds
VerifyFixes                   PASS      0.14 seconds
VerifySignedoff               PASS      0.14 seconds
GitLint                       FAIL      0.40 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      28.38 seconds
CheckAllWarning               PASS      31.35 seconds
CheckSparse                   PASS      30.08 seconds
BuildKernel32                 PASS      27.75 seconds
TestRunnerSetup               PASS      606.70 seconds
TestRunner_l2cap-tester       PASS      64.46 seconds
TestRunner_iso-tester         PASS      93.80 seconds
TestRunner_bnep-tester        PASS      19.81 seconds
TestRunner_mgmt-tester        FAIL      220.39 seconds
TestRunner_rfcomm-tester      PASS      26.70 seconds
TestRunner_sco-tester         PASS      33.44 seconds
TestRunner_ioctl-tester       PASS      27.01 seconds
TestRunner_mesh-tester        FAIL      26.98 seconds
TestRunner_smp-tester         PASS      23.79 seconds
TestRunner_userchan-tester    PASS      20.52 seconds
TestRunner_6lowpan-tester     PASS      23.05 seconds
IncrementalBuild              PASS      32.28 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#146: 
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8

total: 0 errors, 1 warnings, 0 checks, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14605450.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()

11: B3 Line contains hard tab characters (\t): "	if (adv) {"
12: B3 Line contains hard tab characters (\t): "		memcpy(ptr, adv->adv_data, adv->adv_data_len);"
25: B1 Line exceeds max length (82>80): "  BUG: KASAN: stack-out-of-bounds in eir_create_adv_data (net/bluetooth/eir.c:302)"
##############################
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.257 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    2.782 seconds
Mesh - Send cancel - 2                               Timed out    1.991 seconds


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

---
Regards,
Linux Bluetooth


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

* Re: [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()
  2026-06-01 17:38 ` Luiz Augusto von Dentz
@ 2026-06-02 16:14   ` Weiming Shi
  0 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-06-02 16:14 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: linux-bluetooth, Marcel Holtmann, linux-kernel, Xiang Mei

On 26-06-01 13:38, Luiz Augusto von Dentz wrote:
> Hi Xiang,
> 
> On Mon, Jun 1, 2026 at 12:22 PM Xiang Mei <bestswngs@gmail.com> wrote:
> >
> > From: Weiming Shi <bestswngs@gmail.com>
> >
> > eir_create_adv_data() builds the advertising data into a fixed-size
> > buffer of "size" bytes (31 for the legacy path in hci_set_adv_data_sync()).
> > It may prepend a 3-byte "Flags" AD structure, but the per-instance
> > advertising data is then copied without checking that it still fits:
> >
> > skip_flags:
> >         if (adv) {
> >                 memcpy(ptr, adv->adv_data, adv->adv_data_len);
> >
> > The "Flags" structure is added whenever BR/EDR is disabled
> > (LE_AD_NO_BREDR), which is the normal state for an LE-only controller.
> > However, the mgmt length validator tlv_data_max_len() only reserves
> > those 3 bytes when the user-supplied adv_flags carries a managed-flags
> > bit (DISCOV / LIMITED_DISCOV / MANAGED_FLAGS). Adding an instance with
> > flags == 0 reserves nothing, so adv_data_len is accepted up to the full
> > buffer size. At advertise time the 3 flag bytes are still prepended,
> > and the subsequent memcpy() writes 3 + adv_data_len bytes into the
> > size-byte buffer, overflowing it by the attacker-controlled tail of
> > adv_data:
> >
> >   BUG: KASAN: stack-out-of-bounds in eir_create_adv_data (net/bluetooth/eir.c:302)
> >   Write of size 31 at addr ffff88800b7e7bac by task kworker/u9:0/51
> >   Workqueue: hci0 hci_cmd_sync_work
> >    __asan_memcpy (mm/kasan/shadow.c:106)
> >    eir_create_adv_data (net/bluetooth/eir.c:302)
> >    hci_update_adv_data_sync (net/bluetooth/hci_sync.c:1689)
> >    hci_schedule_adv_instance_sync (net/bluetooth/hci_sync.c:2015)
> >    hci_cmd_sync_work (net/bluetooth/hci_sync.c:332)
> >   This frame has 1 object:
> >    [32, 64) 'cp'
> 
> Add the btmon trace of the MGMT command when this is triggered, and
> explaing how the advertisement was created, as with bluetoothd?
> 
> > The inconsistency dates back to when the managed "Flags" field was first
> > added to the Add Advertising path: the prepended LE_AD_NO_BREDR flag does
> > not depend on the user-supplied adv_flags, but tlv_data_is_valid() only
> > reserved room when MGMT_ADV_FLAG_DISCOV was set. Commit 47c03902269a
> > ("Bluetooth: eir: Fix possible crashes on eir_create_adv_data") later
> > added the "size" argument and bounds-checked the "Flags" and "Tx Power"
> > AD structures, but left this copy unguarded. Guard it the same way so
> > the data is only copied when it fits.
> >
> > The bug is reachable by a local user with CAP_NET_ADMIN that owns an
> > LE-only controller using the legacy advertising path.
> >
> > Fixes: b44133ff03be ("Bluetooth: Support the "discoverable" adv flag")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  net/bluetooth/eir.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/bluetooth/eir.c b/net/bluetooth/eir.c
> > index 3f72111ba651..e574f8f61e16 100644
> > --- a/net/bluetooth/eir.c
> > +++ b/net/bluetooth/eir.c
> > @@ -297,7 +297,7 @@ u8 eir_create_adv_data(struct hci_dev *hdev, u8 instance, u8 *ptr, u8 size)
> >         }
> >
> >  skip_flags:
> > -       if (adv) {
> > +       if (adv && ad_len + adv->adv_data_len <= size) {
> 
> So we have 2 options: 1) Don't add flags if there is no space, or 2)
> Don't add the user provided data. We should probably choose option 1,
> not option 2 since option 2 probably means the advertisement is
> useless.
> 
> >                 memcpy(ptr, adv->adv_data, adv->adv_data_len);
> >                 ad_len += adv->adv_data_len;
> >                 ptr += adv->adv_data_len;
> > --
> > 2.43.0
> >
> 
> 
> -- 
> Luiz Augusto von Dentz

Thanks. Agreed on option 1. I'll send v2 implementing that shortly.

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

end of thread, other threads:[~2026-06-02 16:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-01 16:22 [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data() Xiang Mei
2026-06-01 17:38 ` Luiz Augusto von Dentz
2026-06-02 16:14   ` Weiming Shi
2026-06-01 18:59 ` bluez.test.bot

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