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

[Luiz Augusto von Dentz]

Hi Xiang,

On Mon, Jun 1, 2026 at 12:22 PM Xiang Mei <bestswngs@xxxxxxxxx> wrote:
>
> From: Weiming Shi <bestswngs@xxxxxxxxx>
>
> 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@xxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
> ---
>  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