Commit 728abc01 authored by Niels Dossche's avatar Niels Dossche Committed by Marcel Holtmann

Bluetooth: hci_event: Add missing locking on hdev in hci_le_ext_adv_term_evt

Both hci_find_adv_instance and hci_remove_adv_instance have a comment
above their function definition saying that these two functions require
the caller to hold the hdev->lock lock. However, hci_le_ext_adv_term_evt
does not acquire that lock and neither does its caller hci_le_meta_evt
(hci_le_meta_evt calls hci_le_ext_adv_term_evt via an indirect function
call because of the lookup in hci_le_ev_table).

The other event handlers all acquire and release the hdev->lock and they
follow the rule that hci_find_adv_instance and hci_remove_adv_instance
must be called while holding the hdev->lock lock.

The solution is to make sure hci_le_ext_adv_term_evt also acquires and
releases the hdev->lock lock. The check on ev->status which logs a
warning and does an early return is not covered by the lock because
other functions also access ev->status without holding the lock.
Signed-off-by: default avatarNiels Dossche <niels.dossche@ugent.be>
Signed-off-by: default avatarMarcel Holtmann <marcel@holtmann.org>
parent f95fd5f3
...@@ -5716,8 +5716,6 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, ...@@ -5716,8 +5716,6 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
adv = hci_find_adv_instance(hdev, ev->handle);
/* The Bluetooth Core 5.3 specification clearly states that this event /* The Bluetooth Core 5.3 specification clearly states that this event
* shall not be sent when the Host disables the advertising set. So in * shall not be sent when the Host disables the advertising set. So in
* case of HCI_ERROR_CANCELLED_BY_HOST, just ignore the event. * case of HCI_ERROR_CANCELLED_BY_HOST, just ignore the event.
...@@ -5730,9 +5728,13 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, ...@@ -5730,9 +5728,13 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
return; return;
} }
hci_dev_lock(hdev);
adv = hci_find_adv_instance(hdev, ev->handle);
if (ev->status) { if (ev->status) {
if (!adv) if (!adv)
return; goto unlock;
/* Remove advertising as it has been terminated */ /* Remove advertising as it has been terminated */
hci_remove_adv_instance(hdev, ev->handle); hci_remove_adv_instance(hdev, ev->handle);
...@@ -5740,12 +5742,12 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, ...@@ -5740,12 +5742,12 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
list_for_each_entry_safe(adv, n, &hdev->adv_instances, list) { list_for_each_entry_safe(adv, n, &hdev->adv_instances, list) {
if (adv->enabled) if (adv->enabled)
return; goto unlock;
} }
/* We are no longer advertising, clear HCI_LE_ADV */ /* We are no longer advertising, clear HCI_LE_ADV */
hci_dev_clear_flag(hdev, HCI_LE_ADV); hci_dev_clear_flag(hdev, HCI_LE_ADV);
return; goto unlock;
} }
if (adv) if (adv)
...@@ -5760,16 +5762,19 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data, ...@@ -5760,16 +5762,19 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
if (hdev->adv_addr_type != ADDR_LE_DEV_RANDOM || if (hdev->adv_addr_type != ADDR_LE_DEV_RANDOM ||
bacmp(&conn->resp_addr, BDADDR_ANY)) bacmp(&conn->resp_addr, BDADDR_ANY))
return; goto unlock;
if (!ev->handle) { if (!ev->handle) {
bacpy(&conn->resp_addr, &hdev->random_addr); bacpy(&conn->resp_addr, &hdev->random_addr);
return; goto unlock;
} }
if (adv) if (adv)
bacpy(&conn->resp_addr, &adv->random_addr); bacpy(&conn->resp_addr, &adv->random_addr);
} }
unlock:
hci_dev_unlock(hdev);
} }
static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, void *data, static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, void *data,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment