0197314b38f3d50156a7affe16a9d0c2c27ea21b4916003973916e0e.patch

From d1dc8557f5d757c240b7e92838135d03d1d0b7ab Mon Sep 17 00:00:00 2001
From: Daniel Winkler <danielwinkler@google.com>
Date: Wed, 10 Feb 2021 13:52:05 -0800
Subject: [PATCH] CHROMIUM: Track adv state correctly when extended adv
 available

We have discovered that in some use cases, the HCI_LE_ADV will not
correctly reflect the advertising state when extended advertising is
available. Due to the logic from pre-extended implementation, when an
"enable advertising" command is received with enable=1, HCI_LE_ADV is
set to True, and if enable=0, HCI_LE_ADV is set to false. However, as
ext_adv allows multiple concurrent advertisements, this logic is now
incorrect. Consider the following sequence of bluez events:

1. RegisterAdvertisement(adv1)
2. RegisterAdvertisement(adv2)
3. UnregisterAdvertisement(adv2)

In step 3, HCI_LE_ADV is set to false even though adv1 is still active.
This patch does the following to improve correctness of the HCI_LE_ADV
flag:

* Since directed advertisements are not tracked via the
  hdev->adv_instances list, add a new hdev->ext_directed_advertising
  bool to track when directed advertising is occurring.
* Since extended advertising now has a new hci_le_ext_adv_term_evt event
  that fires when the controller decides to cancel an advertisement due
  to a timeout, a connect event to that adv's address, etc., allow
  HCI_LE_ADV to be updated here as well.
* Since hci_le_ext_adv_term_evt cancels an adv, we now remove this adv
  from hdev->adv_instances.

Note that in the third point here, we remove the cancelled adv from
hdev->adv_instances, but this patch does not handle the propagation to
userspace (and client). This patch simply syncs the kernel to the
controller's adv state, which is necessary to have an accurate count of
active advertisements.

The patch is tested with our QuickHealth and AdvHealth automated test
suites. Additionally, it is tested by performing the above steps and
then validating that the HCI_LE_ADV flag correctly represents the
controller state.

Signed-off-by: Daniel Winkler <danielwinkler@google.com>

BUG=b:181049961
TEST=AdvHealth.adv_nearby_test on Ezkinil

Change-Id: I3a0f06b0eaafff9bece6e073326b8986a3098532
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2733175
Reviewed-by: Alain Michaud <alainm@chromium.org>
Commit-Queue: Daniel Winkler <danielwinkler@google.com>
Tested-by: Daniel Winkler <danielwinkler@google.com>
---
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_core.c         |  1 +
 net/bluetooth/hci_event.c        | 24 ++++++++++++++++++++++--
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 30ed9e56a5ed..5f3fb67e8d73 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -587,6 +587,8 @@ struct hci_dev {
 	__u8			cur_adv_instance;
 	__u16			adv_instance_timeout;
 	struct delayed_work	adv_instance_expire;
+	/* We can have a single directed advertisement using adv handle 0 */
+	bool			ext_directed_advertising;
 
 	struct idr		adv_monitors_idr;
 	unsigned int		adv_monitors_cnt;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1cf1ea7d32d5..8ae1907ec000 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2431,6 +2431,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
 	hdev->adv_instance_cnt = 0;
 	hdev->cur_adv_instance = 0x00;
 	hdev->adv_instance_timeout = 0;
+	hdev->ext_directed_advertising = false;
 	hdev->eir_max_name_len = 48;
 
 	hdev->advmon_allowlist_duration = 300;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index abaabfae19cc..5f59d11c7b6b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5763,17 +5763,37 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 
 		if (hdev->adv_addr_type != ADDR_LE_DEV_RANDOM ||
 		    bacmp(&conn->resp_addr, BDADDR_ANY))
-			goto unlock;
+			goto cleanup_instance;
 
 		if (!ev->handle) {
 			bacpy(&conn->resp_addr, &hdev->random_addr);
-			goto unlock;
+			goto cleanup_instance;
 		}
 
 		if (adv)
 			bacpy(&conn->resp_addr, &adv->random_addr);
 	}
 
+cleanup_instance:
+	/* Since the controller tells us this instance is no longer active, we
+	 * remove it.
+	 *
+	 * One caveat is that if the status is HCI_ERROR_CANCELLED_BY_HOST, this
+	 * event is being fired as a result of a hci_cp_le_set_ext_adv_enable
+	 * disable request, which will have its own callback and cleanup via
+	 * the hci_cc_le_set_ext_adv_enable path. If this is the case, we will
+	 * not remove the instance, as it may only be a temporary disable.
+	 */
+	if (ev->status != HCI_ERROR_CANCELLED_BY_HOST) {
+		hci_remove_adv_instance(hdev, ev->handle);
+
+		/* If we are no longer advertising, clear HCI_LE_ADV */
+		if (list_empty(&hdev->adv_instances) &&
+		    !hdev->ext_directed_advertising) {
+			hci_dev_clear_flag(hdev, HCI_LE_ADV);
+		}
+	}
+
 unlock:
 	hci_dev_unlock(hdev);
 }
-- 
2.35.0