[PATCH v2 1/1] Bluetooth: hci_sync: drop adv instances the controller cannot carry

[Piyus Kedia]

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@xxxxxxxxxxxxxxx
Co-developed-by: Kapil Vaswani <kapil@xxxxxxxxxxxxxxx>
Signed-off-by: Kapil Vaswani <kapil@xxxxxxxxxxxxxxx>
Signed-off-by: Piyus Kedia <piyus@xxxxxxxxxxx>
---
 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