Re: [PATCH] Bluetooth: HCI: Fix HCI command order for extended advertising

Hi Christian,

On Wed, Jun 25, 2025 at 9:05 AM Christian Eggers ceggers@arri.de wrote:

For extended advertising capable controllers, hci_start_ext_adv_sync() at the moment synchronously calls SET_EXT_ADV_PARAMS [1], SET_ADV_SET_RAND_ADDR [2], SET_EXT_SCAN_RSP_DATA [3](optional) and SET_EXT_ADV_ENABLE [4]. After all synchronous commands are finished, SET_EXT_ADV_DATA is called from the async response handler of SET_EXT_ADV_PARAMS [5] (via hci_update_adv_data).

So the current implementation sets the advertising data AFTER enabling the advertising instance. The BT Core specification explicitly allows for this [6]:

...

If advertising is currently enabled for the specified advertising set, the Controller shall use the new data in subsequent extended advertising events for this advertising set. If an extended advertising event is in progress when this command is issued, the Controller may use the old or new data for that event.

Ok, lets stop right here, if the controller deviates from the spec it needs a quirk and not make the whole stack work around a bug in the firmware.

In case of the Realtek RTL8761BU chip (almost all contemporary BT USB dongles are built on it), updating the advertising data after enabling the instance produces (at least one) corrupted advertising message. Under normal conditions, a single corrupted advertising message would probably not attract much attention, but during MESH provisioning (via MGMT I/O / mesh_send(_sync)), up to 3 different messages (BEACON, ACK, CAPS) are sent within a loop which causes corruption of ALL provisioning messages.

I have no idea whether this could be fixed in the firmware of the USB dongles (I didn't even find the chip on the Realtek homepage), but generally I would suggest changing the order of the HCI commands as this matches the command order for "non-extended adv capable" controllers and simply is more natural.

This patch only considers advertising instances with handle > 0, I don't know whether this should be extended to further cases.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/... [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/... [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/... [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/... [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/... [6] https://www.bluetooth.com/wp-content/uploads/Files/Specification/HTML/Core-6...

Signed-off-by: Christian Eggers ceggers@arri.de Fixes: a0fb3726ba55 ("Bluetooth: Use Set ext adv/scan rsp data if controller supports") Cc: stable@vger.kernel.org

---

include/net/bluetooth/hci_core.h | 1 + include/net/bluetooth/hci_sync.h | 1 + net/bluetooth/hci_event.c | 33 +++++++++++++++++++++++++++++ net/bluetooth/hci_sync.c | 36 ++++++++++++++++++++++++++------ 4 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 9fc8f544e20e..8d37f127ddba 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -237,6 +237,7 @@ struct oob_data {

struct adv_info { struct list_head list;

•   bool    enable_after_set_ext_data;
    bool    enabled;
    bool    pending;
    bool    periodic;
  

diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index 5224f57f6af2..00eceffeec87 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -112,6 +112,7 @@ int hci_schedule_adv_instance_sync(struct hci_dev *hdev, u8 instance, int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance); int hci_start_ext_adv_sync(struct hci_dev *hdev, u8 instance); int hci_enable_ext_advertising_sync(struct hci_dev *hdev, u8 instance); +int hci_enable_ext_advertising(struct hci_dev *hdev, u8 instance); int hci_enable_advertising_sync(struct hci_dev *hdev); int hci_enable_advertising(struct hci_dev *hdev);

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 66052d6aaa1d..eb018d8a3c4b 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2184,6 +2184,37 @@ static u8 hci_cc_set_ext_adv_param(struct hci_dev *hdev, void *data, return rp->status; }

+static u8 hci_cc_le_set_ext_adv_data(struct hci_dev *hdev, void *data,

•                                struct sk_buff *skb)
  

+{

•   struct hci_cp_le_set_ext_adv_data *cp;
  

•   struct hci_ev_status *rp = data;
  

•   struct adv_info *adv_instance;
  

•   bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
  

•   if (rp->status)
  

•           return rp->status;
  

•   cp = hci_sent_cmd_data(hdev, HCI_OP_LE_SET_EXT_ADV_DATA);
  

•   if (!cp)
  

•           return rp->status;
  

•   hci_dev_lock(hdev);
  

•   if (cp->handle) {
  

•           adv_instance = hci_find_adv_instance(hdev, cp->handle);
  

•           if (adv_instance) {
  

•                   if (adv_instance->enable_after_set_ext_data)
  

•                           hci_enable_ext_advertising(hdev, cp->handle);
  

•           }
  

•   }
  

•   hci_dev_unlock(hdev);
  

•   return rp->status;
  

+}

static u8 hci_cc_read_rssi(struct hci_dev *hdev, void *data, struct sk_buff *skb) { @@ -4166,6 +4197,8 @@ static const struct hci_cc { sizeof(struct hci_rp_le_read_num_supported_adv_sets)), HCI_CC(HCI_OP_LE_SET_EXT_ADV_PARAMS, hci_cc_set_ext_adv_param, sizeof(struct hci_rp_le_set_ext_adv_params)),

•   HCI_CC_STATUS(HCI_OP_LE_SET_EXT_ADV_DATA,
  

•                 hci_cc_le_set_ext_adv_data),
    HCI_CC_STATUS(HCI_OP_LE_SET_EXT_ADV_ENABLE,
                  hci_cc_le_set_ext_adv_enable),
    HCI_CC_STATUS(HCI_OP_LE_SET_ADV_SET_RAND_ADDR,
  

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index 1f8806dfa556..da0e39cce721 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -1262,6 +1262,7 @@ int hci_setup_ext_adv_instance_sync(struct hci_dev *hdev, u8 instance) hci_cpu_to_le24(adv->max_interval, cp.max_interval); cp.tx_power = adv->tx_power; cp.sid = adv->sid;

•           adv->enable_after_set_ext_data = true;
    } else {
            hci_cpu_to_le24(hdev->le_adv_min_interval, cp.min_interval);
            hci_cpu_to_le24(hdev->le_adv_max_interval, cp.max_interval);
  

@@ -1456,6 +1457,23 @@ int hci_enable_ext_advertising_sync(struct hci_dev *hdev, u8 instance) data, HCI_CMD_TIMEOUT); }

+static int enable_ext_advertising_sync(struct hci_dev *hdev, void *data) +{

•   u8 instance = PTR_UINT(data);
  

•   return hci_enable_ext_advertising_sync(hdev, instance);
  

+}

+int hci_enable_ext_advertising(struct hci_dev *hdev, u8 instance) +{

•   if (!hci_dev_test_flag(hdev, HCI_ADVERTISING) &&
  

•       list_empty(&hdev->adv_instances))
  

•           return 0;
  

•   return hci_cmd_sync_queue(hdev, enable_ext_advertising_sync,
  

•                             UINT_PTR(instance), NULL);
  

+}

int hci_start_ext_adv_sync(struct hci_dev *hdev, u8 instance) { int err; @@ -1464,11 +1482,11 @@ int hci_start_ext_adv_sync(struct hci_dev *hdev, u8 instance) if (err) return err;

•   err = hci_set_ext_scan_rsp_data_sync(hdev, instance);
  

•   if (err)
  

•           return err;
  

•   return hci_enable_ext_advertising_sync(hdev, instance);
  

•   /* SET_EXT_ADV_DATA and SET_EXT_ADV_ENABLE are called in the
  

•    * asynchronous response chain of set_ext_adv_params in order to
  

•    * set the advertising data first prior enabling it.
  

•    */
  

Doing things asynchronously is known to create problems, which is why we introduced the cmd_sync infra to handle a chain of commands like this, so Id suggest sticking to the synchronous way, if the order needs to be changed then use a quirk to detect it and then make sure the instance is disabled on hci_set_ext_adv_data_sync and then re-enable after updating it.

•   return hci_set_ext_scan_rsp_data_sync(hdev, instance);
  

}

int hci_disable_per_advertising_sync(struct hci_dev *hdev, u8 instance) @@ -1832,8 +1850,14 @@ static int hci_set_ext_adv_data_sync(struct hci_dev *hdev, u8 instance)

    if (instance) {
            adv = hci_find_adv_instance(hdev, instance);

•           if (!adv || !adv->adv_data_changed)
  

•           if (!adv)
                    return 0;
  

•           if (!adv->adv_data_changed) {
  

•                   if (adv->enable_after_set_ext_data)
  

•                           hci_enable_ext_advertising_sync(hdev,
  

•                                                           adv->handle);
  

•                   return 0;
  

•           }
    }
  
    len = eir_create_adv_data(hdev, instance, pdu->data,
  

-- 2.43.0