Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()

[PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()

Posted by Xiang Mei 6 days, 12 hours ago

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

Re: [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()

Posted by Luiz Augusto von Dentz 6 days, 10 hours ago

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

Re: [PATCH bluetooth] Bluetooth: eir: Fix stack OOB write in eir_create_adv_data()

Posted by Weiming Shi 5 days, 12 hours ago

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.