Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH v2 0/1] Bluetooth: fix stack OOB write in the legacy adv data path
@ 2026-11-10 17:53 Piyus Kedia
  2026-11-10 17:53 ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Piyus Kedia
  0 siblings, 1 reply; 5+ messages in thread
From: Piyus Kedia @ 2026-11-10 17:53 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, linux-kernel, Kapil Vaswani, Piyus Kedia

Hi,

We would like to follow up on the stack out-of-bounds writes in
eir_create_adv_data() and eir_create_scan_rsp() we reported earlier.

The patch we suggested earlier bounded the copy in eir.c. As Luiz pointed
out, the stale advertising instance is the bug rather than the missing
bounds check, so this new patch unregisters instances that no longer fit
the controller's capability, at power-on, and reports them with
MGMT_EV_ADVERTISING_REMOVED. Userspace can add them again with the right
size.

Verified at runtime: with this patch applied, the reproducer completes
cleanly on both the adv_data and scan_rsp paths and the instance is
removed; without it KASAN reports a 220-byte write into the 31-byte
buffer at eir.c:303 and eir.c:360. The tree it was verified on has
net/bluetooth/eir.c and hci_powered_update_adv_sync() byte-identical to
this base.

Apologies for the form of our earlier submission: it was a bug report
that patchwork picked up as a patch, which is why the whole report ended
up as the commit message.

Link to v1: https://patchwork.kernel.org/series/1206772/

Thanks and regards,
Piyus Kedia
Kapil Vaswani

Piyus Kedia (1):
  Bluetooth: hci_sync: drop adv instances the controller cannot carry

 net/bluetooth/hci_sync.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

-- 
2.34.1


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

* [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry
  2026-11-10 17:53 [PATCH v2 0/1] Bluetooth: fix stack OOB write in the legacy adv data path Piyus Kedia
@ 2026-11-10 17:53 ` Piyus Kedia
  2026-11-10 18:59   ` Bluetooth: fix stack OOB write in the legacy adv data path bluez.test.bot
  2026-11-11 19:49   ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Luiz Augusto von Dentz
  0 siblings, 2 replies; 5+ messages in thread
From: Piyus Kedia @ 2026-11-10 17:53 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, linux-kernel, Kapil Vaswani, Piyus Kedia, stable

An advertising instance stores up to HCI_MAX_EXT_AD_LENGTH (251) bytes
in adv_info. How much of that is accepted when the instance is added is
decided by max_adv_len(), which depends on a feature bit in the
controller's LE Extended Advertising support:

  ext_adv_capable(dev) ? HCI_MAX_EXT_AD_LENGTH : HCI_MAX_AD_LENGTH

ext_adv_capable() reads hdev->le_features[1], which is re-read from the
controller on every bring-up, while the instance list is only freed in
hci_release_dev(). Instances therefore outlive a power cycle, and an
instance added while the controller reported LE Extended Advertising
can be far larger than the capability now allows.

hci_update_adv_data_sync() picks the writing path by that same bit.
Without it the legacy path runs, and hci_set_adv_data_sync() writes
into struct hci_cp_le_set_adv_data, whose data[] is HCI_MAX_AD_LENGTH
(31) bytes on the stack, while eir_create_adv_data() copies
adv->adv_data_len bytes into it. A controller that reports the feature
on one bring-up and not on the next overflows that buffer. This can
happen when, for example, a USB dongle answers LE Read Local Features
differently after a reset. scan_rsp_len has the same problem against
hci_cp_le_set_scan_rsp_data.

  BUG: KASAN: stack-out-of-bounds in eir_create_adv_data+0x2ab/0x440
  Write of size 220 at addr ffffc90006b679c1 by task syz-executor

  BUG: KASAN: stack-out-of-bounds in eir_create_scan_rsp+0x119/0x1a0
  Write of size 220 at addr ffffc90006ca79c1 by task syz-executor

Both are reached from HCIDEVUP by way of hci_powered_update_adv_sync()
re-arming a stored instance.

Bounding the copy in eir.c would leave the instance registered and
unusable, to be retried on every bring-up. Unregister it instead, once
the features have been re-read and before anything is scheduled, and
report it with MGMT_EV_ADVERTISING_REMOVED. Userspace can add the
instance again at a size this controller will accept. Dropping the
instances before the list_empty() test below also lets the default
instance be set up.

The removal is under hci_dev_lock() because
hci_schedule_adv_instance_sync() sends commands, which cannot be done
with the lock held.

Fixes: 112b5090c219 ("Bluetooth: MGMT: Fix always using HCI_MAX_AD_LENGTH")
Cc: stable@vger.kernel.org
Co-developed-by: Kapil Vaswani <kapil@presceralabs.ai>
Signed-off-by: Kapil Vaswani <kapil@presceralabs.ai>
Signed-off-by: Piyus Kedia <piyus@iiitd.ac.in>
---
 net/bluetooth/hci_sync.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 9213e3ef9f26..ee8b1cd9dee5 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -3608,6 +3608,34 @@ static int hci_powered_update_adv_sync(struct hci_dev *hdev)
 	if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
 		return 0;
 
+	/* An instance's data was accepted against max_adv_len() as it was
+	 * when the instance was added, and that is a branch on a controller
+	 * feature which has just been re-read. Anything too large for the
+	 * current capability can no longer be rendered: drop it and tell
+	 * userspace, so it can add the instance again at a size this
+	 * controller will take.
+	 *
+	 * This is safe as long as there is no command send while the lock is
+	 * held.
+	 */
+	hci_dev_lock(hdev);
+
+	list_for_each_entry_safe(adv, tmp, &hdev->adv_instances, list) {
+		u8 instance = adv->instance;
+
+		if (adv->adv_data_len <= max_adv_len(hdev) &&
+		    adv->scan_rsp_len <= max_adv_len(hdev))
+			continue;
+
+		bt_dev_warn(hdev, "instance %u too large for this controller",
+			    instance);
+
+		if (!hci_remove_adv_instance(hdev, instance))
+			mgmt_advertising_removed(NULL, hdev, instance);
+	}
+
+	hci_dev_unlock(hdev);
+
 	/* If RPA Resolution has not been enable yet it means the
 	 * resolving list is empty and we should attempt to program the
 	 * local IRK in order to support using own_addr_type
-- 
2.34.1


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

* RE: Bluetooth: fix stack OOB write in the legacy adv data path
  2026-11-10 17:53 ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Piyus Kedia
@ 2026-11-10 18:59   ` bluez.test.bot
  2026-11-11 19:49   ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Luiz Augusto von Dentz
  1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2026-11-10 18:59 UTC (permalink / raw)
  To: linux-bluetooth, piyus

[-- Attachment #1: Type: text/plain, Size: 1605 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/series/1207528/

---Test result---

Test Summary:
CheckPatch                    PASS      0.53 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.20 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      24.04 seconds
CheckAllWarning               PASS      27.41 seconds
CheckSparse                   PASS      29.59 seconds
BuildKernel32                 PASS      24.15 seconds
CheckKernelLLVM               PASS      26.22 seconds
TestRunnerSetup               PASS      554.53 seconds
TestRunner_l2cap-tester       PASS      13.20 seconds
TestRunner_iso-tester         PASS      9.60 seconds
TestRunner_bnep-tester        PASS      2.86 seconds
TestRunner_mgmt-tester        PASS      84.44 seconds
TestRunner_rfcomm-tester      PASS      4.22 seconds
TestRunner_sco-tester         PASS      5.17 seconds
TestRunner_ioctl-tester       PASS      4.29 seconds
TestRunner_mesh-tester        PASS      5.16 seconds
TestRunner_smp-tester         PASS      4.04 seconds
TestRunner_userchan-tester    PASS      2.93 seconds
TestRunner_6lowpan-tester     PASS      4.15 seconds
IncrementalBuild              PASS      23.04 seconds



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

---
Regards,
Linux Bluetooth


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

* Re: [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry
  2026-11-10 17:53 ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Piyus Kedia
  2026-11-10 18:59   ` Bluetooth: fix stack OOB write in the legacy adv data path bluez.test.bot
@ 2026-11-11 19:49   ` Luiz Augusto von Dentz
  2026-11-12 12:57     ` Piyus Kedia
  1 sibling, 1 reply; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2026-11-11 19:49 UTC (permalink / raw)
  To: Piyus Kedia
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, Kapil Vaswani,
	stable

Hi Piyus,

On Tue, Nov 10, 2026 at 12:54 PM Piyus Kedia <piyus@iiitd.ac.in> wrote:
>
> An advertising instance stores up to HCI_MAX_EXT_AD_LENGTH (251) bytes
> in adv_info. How much of that is accepted when the instance is added is
> decided by max_adv_len(), which depends on a feature bit in the
> controller's LE Extended Advertising support:
>
>   ext_adv_capable(dev) ? HCI_MAX_EXT_AD_LENGTH : HCI_MAX_AD_LENGTH
>
> ext_adv_capable() reads hdev->le_features[1], which is re-read from the
> controller on every bring-up, while the instance list is only freed in
> hci_release_dev(). Instances therefore outlive a power cycle, and an
> instance added while the controller reported LE Extended Advertising
> can be far larger than the capability now allows.
>
> hci_update_adv_data_sync() picks the writing path by that same bit.
> Without it the legacy path runs, and hci_set_adv_data_sync() writes
> into struct hci_cp_le_set_adv_data, whose data[] is HCI_MAX_AD_LENGTH
> (31) bytes on the stack, while eir_create_adv_data() copies
> adv->adv_data_len bytes into it. A controller that reports the feature
> on one bring-up and not on the next overflows that buffer. This can
> happen when, for example, a USB dongle answers LE Read Local Features
> differently after a reset. scan_rsp_len has the same problem against
> hci_cp_le_set_scan_rsp_data.
>
>   BUG: KASAN: stack-out-of-bounds in eir_create_adv_data+0x2ab/0x440
>   Write of size 220 at addr ffffc90006b679c1 by task syz-executor
>
>   BUG: KASAN: stack-out-of-bounds in eir_create_scan_rsp+0x119/0x1a0
>   Write of size 220 at addr ffffc90006ca79c1 by task syz-executor
>
> Both are reached from HCIDEVUP by way of hci_powered_update_adv_sync()
> re-arming a stored instance.
>
> Bounding the copy in eir.c would leave the instance registered and
> unusable, to be retried on every bring-up. Unregister it instead, once
> the features have been re-read and before anything is scheduled, and
> report it with MGMT_EV_ADVERTISING_REMOVED. Userspace can add the
> instance again at a size this controller will accept. Dropping the
> instances before the list_empty() test below also lets the default
> instance be set up.
>
> The removal is under hci_dev_lock() because
> hci_schedule_adv_instance_sync() sends commands, which cannot be done
> with the lock held.
>
> Fixes: 112b5090c219 ("Bluetooth: MGMT: Fix always using HCI_MAX_AD_LENGTH")
> Cc: stable@vger.kernel.org
> Co-developed-by: Kapil Vaswani <kapil@presceralabs.ai>
> Signed-off-by: Kapil Vaswani <kapil@presceralabs.ai>
> Signed-off-by: Piyus Kedia <piyus@iiitd.ac.in>
> ---
>  net/bluetooth/hci_sync.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 9213e3ef9f26..ee8b1cd9dee5 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -3608,6 +3608,34 @@ static int hci_powered_update_adv_sync(struct hci_dev *hdev)
>         if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
>                 return 0;
>
> +       /* An instance's data was accepted against max_adv_len() as it was
> +        * when the instance was added, and that is a branch on a controller
> +        * feature which has just been re-read. Anything too large for the
> +        * current capability can no longer be rendered: drop it and tell
> +        * userspace, so it can add the instance again at a size this
> +        * controller will take.
> +        *
> +        * This is safe as long as there is no command send while the lock is
> +        * held.
> +        */
> +       hci_dev_lock(hdev);
> +
> +       list_for_each_entry_safe(adv, tmp, &hdev->adv_instances, list) {
> +               u8 instance = adv->instance;
> +
> +               if (adv->adv_data_len <= max_adv_len(hdev) &&
> +                   adv->scan_rsp_len <= max_adv_len(hdev))
> +                       continue;
> +
> +               bt_dev_warn(hdev, "instance %u too large for this controller",
> +                           instance);
> +
> +               if (!hci_remove_adv_instance(hdev, instance))
> +                       mgmt_advertising_removed(NULL, hdev, instance);
> +       }
> +
> +       hci_dev_unlock(hdev);
> +
>         /* If RPA Resolution has not been enable yet it means the
>          * resolving list is empty and we should attempt to program the
>          * local IRK in order to support using own_addr_type
> --
> 2.34.1

Sashiko flagged a few problems:

https://sashiko.dev/#/patchset/20261110175334.676816-1-piyus%40iiitd.ac.in

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry
  2026-11-11 19:49   ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Luiz Augusto von Dentz
@ 2026-11-12 12:57     ` Piyus Kedia
  0 siblings, 0 replies; 5+ messages in thread
From: Piyus Kedia @ 2026-11-12 12:57 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, Kapil Vaswani,
	stable

Hi Luiz,

We just submitted another version that addressed the issues raised by Sashiko.

https://lore.kernel.org/linux-bluetooth/20261112125420.3905719-1-piyus@iiitd.ac.in/T/#u

Best,
Piyus

On Thu, Nov 12, 2026 at 1:19 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Piyus,
>
> On Tue, Nov 10, 2026 at 12:54 PM Piyus Kedia <piyus@iiitd.ac.in> wrote:
> >
> > An advertising instance stores up to HCI_MAX_EXT_AD_LENGTH (251) bytes
> > in adv_info. How much of that is accepted when the instance is added is
> > decided by max_adv_len(), which depends on a feature bit in the
> > controller's LE Extended Advertising support:
> >
> >   ext_adv_capable(dev) ? HCI_MAX_EXT_AD_LENGTH : HCI_MAX_AD_LENGTH
> >
> > ext_adv_capable() reads hdev->le_features[1], which is re-read from the
> > controller on every bring-up, while the instance list is only freed in
> > hci_release_dev(). Instances therefore outlive a power cycle, and an
> > instance added while the controller reported LE Extended Advertising
> > can be far larger than the capability now allows.
> >
> > hci_update_adv_data_sync() picks the writing path by that same bit.
> > Without it the legacy path runs, and hci_set_adv_data_sync() writes
> > into struct hci_cp_le_set_adv_data, whose data[] is HCI_MAX_AD_LENGTH
> > (31) bytes on the stack, while eir_create_adv_data() copies
> > adv->adv_data_len bytes into it. A controller that reports the feature
> > on one bring-up and not on the next overflows that buffer. This can
> > happen when, for example, a USB dongle answers LE Read Local Features
> > differently after a reset. scan_rsp_len has the same problem against
> > hci_cp_le_set_scan_rsp_data.
> >
> >   BUG: KASAN: stack-out-of-bounds in eir_create_adv_data+0x2ab/0x440
> >   Write of size 220 at addr ffffc90006b679c1 by task syz-executor
> >
> >   BUG: KASAN: stack-out-of-bounds in eir_create_scan_rsp+0x119/0x1a0
> >   Write of size 220 at addr ffffc90006ca79c1 by task syz-executor
> >
> > Both are reached from HCIDEVUP by way of hci_powered_update_adv_sync()
> > re-arming a stored instance.
> >
> > Bounding the copy in eir.c would leave the instance registered and
> > unusable, to be retried on every bring-up. Unregister it instead, once
> > the features have been re-read and before anything is scheduled, and
> > report it with MGMT_EV_ADVERTISING_REMOVED. Userspace can add the
> > instance again at a size this controller will accept. Dropping the
> > instances before the list_empty() test below also lets the default
> > instance be set up.
> >
> > The removal is under hci_dev_lock() because
> > hci_schedule_adv_instance_sync() sends commands, which cannot be done
> > with the lock held.
> >
> > Fixes: 112b5090c219 ("Bluetooth: MGMT: Fix always using HCI_MAX_AD_LENGTH")
> > Cc: stable@vger.kernel.org
> > Co-developed-by: Kapil Vaswani <kapil@presceralabs.ai>
> > Signed-off-by: Kapil Vaswani <kapil@presceralabs.ai>
> > Signed-off-by: Piyus Kedia <piyus@iiitd.ac.in>
> > ---
> >  net/bluetooth/hci_sync.c | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index 9213e3ef9f26..ee8b1cd9dee5 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -3608,6 +3608,34 @@ static int hci_powered_update_adv_sync(struct hci_dev *hdev)
> >         if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED))
> >                 return 0;
> >
> > +       /* An instance's data was accepted against max_adv_len() as it was
> > +        * when the instance was added, and that is a branch on a controller
> > +        * feature which has just been re-read. Anything too large for the
> > +        * current capability can no longer be rendered: drop it and tell
> > +        * userspace, so it can add the instance again at a size this
> > +        * controller will take.
> > +        *
> > +        * This is safe as long as there is no command send while the lock is
> > +        * held.
> > +        */
> > +       hci_dev_lock(hdev);
> > +
> > +       list_for_each_entry_safe(adv, tmp, &hdev->adv_instances, list) {
> > +               u8 instance = adv->instance;
> > +
> > +               if (adv->adv_data_len <= max_adv_len(hdev) &&
> > +                   adv->scan_rsp_len <= max_adv_len(hdev))
> > +                       continue;
> > +
> > +               bt_dev_warn(hdev, "instance %u too large for this controller",
> > +                           instance);
> > +
> > +               if (!hci_remove_adv_instance(hdev, instance))
> > +                       mgmt_advertising_removed(NULL, hdev, instance);
> > +       }
> > +
> > +       hci_dev_unlock(hdev);
> > +
> >         /* If RPA Resolution has not been enable yet it means the
> >          * resolving list is empty and we should attempt to program the
> >          * local IRK in order to support using own_addr_type
> > --
> > 2.34.1
>
> Sashiko flagged a few problems:
>
> https://sashiko.dev/#/patchset/20261110175334.676816-1-piyus%40iiitd.ac.in
>
> --
> Luiz Augusto von Dentz

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

end of thread, other threads:[~2026-11-12 12:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10 17:53 [PATCH v2 0/1] Bluetooth: fix stack OOB write in the legacy adv data path Piyus Kedia
2026-11-10 17:53 ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Piyus Kedia
2026-11-10 18:59   ` Bluetooth: fix stack OOB write in the legacy adv data path bluez.test.bot
2026-11-11 19:49   ` [PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry Luiz Augusto von Dentz
2026-11-12 12:57     ` Piyus Kedia

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