From 582e383178d5fe27bfc5f7c076a980ac20482129 Mon Sep 17 00:00:00 2001
From: Archie Pusaka <apusaka@chromium.org>
Date: Fri, 13 Aug 2021 18:02:11 +0800
Subject: [PATCH] CHROMIUM: Bluetooth: Fix receiving
 HCI_LE_Advertising_Set_Terminated event

This event is received when the controller stops advertising,
specifically for these three reasons:
(a) Connection is successfully created (success).
(b) Timeout is reached (error).
(c) Number of advertising events is reached (error).
(*) This event is NOT generated when the host stops the advertisement.
Refer to the BT spec ver 5.3 vol 4 part E sec 7.7.65.18. Note that the
section was revised from BT spec ver 5.0 vol 2 part E sec 7.7.65.18
which was ambiguous about (*).

There are a couple of issues with the current situation:
(1) RTL8822 sends this event when the host stops the advertisement,
    with status = HCI_ERROR_CANCELLED_BY_HOST (due to (*) above).
    This is treated as an error and the advertisement will be removed
    and userspace will be informed via MGMT event.
    On suspend, we also temporarily disable advertisements. However,
    the userspace thinks the advertisements are removed. This causes
    the advHealth tests involving suspend resume to fail since we
    are testing to remove the advertisements after resume, but the
    advertisements are already removed.
(2) When connection is successful, the related advertisement is
    temporarily disabled, as opposed to being removed when other
    errors arise. Our code mishandled this scenario and removes the
    advertisement even on successful connection. This causes this
    advertisement slot goes into a bad state and can't be reused.

This patch solves both issues by checking HCI_ERROR_CANCELLED_BY_HOST
before everything else, and remove advertisements only on failure
case.

Additionally, this patch brings the shape of the code nearer to those
in upstream.

BUG=b:196523329,b:196521730
TEST=AdvHealth pass on Hayato and Sarien, also manually tests issue
     (1) on Hayato and issue (2) on Sarien.

Signed-off-by: Archie Pusaka <apusaka@chromium.org>

Change-Id: I0dad14ce0373c4919f5447fc51b68efe570e5f8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/3112700
Tested-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>
Commit-Queue: Archie Pusaka <apusaka@chromium.org>
---
 net/bluetooth/hci_event.c | 39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5689,6 +5689,15 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 
 	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
 
+	/* Only RTK chips emit HCI_ERROR_CANCELLED_BY_HOST, which they are not
+	 * supposed to do (consult BT spec vol4 Part E sec 7.7.65.18). 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 (ev->status == HCI_ERROR_CANCELLED_BY_HOST)
+		return;
+
 	adv = hci_find_adv_instance(hdev, ev->handle);
 
 	/* The Bluetooth Core 5.3 specification clearly states that this event
@@ -5711,6 +5720,12 @@ static void hci_le_ext_adv_term_evt(struct hci_dev *hdev, void *data,
 		hci_remove_adv_instance(hdev, ev->handle);
 		mgmt_advertising_removed(NULL, 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);
+		}
+
 		list_for_each_entry_safe(adv, n, &hdev->adv_instances, list) {
 			if (adv->enabled)
 				return;
@@ -5733,36 +5748,16 @@ 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 cleanup_instance;
+			return;
 
 		if (!ev->handle) {
 			bacpy(&conn->resp_addr, &hdev->random_addr);
-			goto cleanup_instance;
+			return;
 		}
 
 		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);
-		}
-	}
 }
 
 static void hci_le_conn_update_complete_evt(struct hci_dev *hdev, void *data,
-- 
2.35.0.rc0.227.g00780c9af4-goog