Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 22 Jul 2022 18:23:30 +0800
Message-Id: <20220722182248.1.I20e96c839200bb75cd6af80384f16c8c01498f57@changeid>
Mime-Version: 1.0
Subject: [PATCH] Bluetooth: hci_sync: Use safe loop when adding accept list
From:   Archie Pusaka <apusaka@google.com>
To:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        Marcel Holtmann <marcel@holtmann.org>
Cc:     CrosBT Upstreaming <chromeos-bluetooth-upstreaming@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>,
        Zhengping Jiang <jiangzp@google.com>,
        Michael Sun <michaelfsun@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Johan Hedberg <johan.hedberg@gmail.com>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

From: Archie Pusaka <apusaka@chromium.org>

When in the middle of adding accept list, the userspace can still
remove devices, therefore causing crash if the removed device is
the one being processed.

Use a safe loop mechanism to guard against deletion while iterating
the pending items.

Below is a sample btsnoop log when user enters wrong passkey when
pairing a LE keyboard and the corresponding stacktrace.
@ MGMT Event: Command Complete (0x0001) plen 10
      Add Device (0x0033) plen 7
        Status: Success (0x00)
        LE Address: CA:CA:BD:78:37:F9 (Static)
< HCI Command: LE Add Device To Accept List (0x08|0x0011) plen 7
        Address type: Random (0x01)
        Address: CA:CA:BD:78:37:F9 (Static)
@ MGMT Event: Device Removed (0x001b) plen 7
        LE Address: CA:CA:BD:78:37:F9 (Static)
> HCI Event: Command Complete (0x0e) plen 4
      LE Add Device To Accept List (0x08|0x0011) ncmd 1
        Status: Success (0x00)

[  167.409813] Call trace:
[  167.409983]  hci_le_add_accept_list_sync+0x64/0x26c
[  167.410150]  hci_update_passive_scan_sync+0x5f0/0x6dc
[  167.410318]  add_device_sync+0x18/0x24
[  167.410486]  hci_cmd_sync_work+0xe8/0x150
[  167.410509]  process_one_work+0x140/0x4d0
[  167.410526]  worker_thread+0x134/0x2e4
[  167.410544]  kthread+0x148/0x160
[  167.410562]  ret_from_fork+0x10/0x30

Signed-off-by: Archie Pusaka <apusaka@chromium.org>
Reviewed-by: Zhengping Jiang <jiangzp@google.com>
Reviewed-by: Michael Sun <michaelfsun@google.com>

---

 net/bluetooth/hci_sync.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 3067d94e7a8e..8e843d34f7de 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -1863,7 +1863,7 @@ struct sk_buff *hci_read_local_oob_data_sync(struct hci_dev *hdev,
  */
 static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 {
-	struct hci_conn_params *params;
+	struct hci_conn_params *params, *tmp;
 	struct bdaddr_list *b, *t;
 	u8 num_entries = 0;
 	bool pend_conn, pend_report;
@@ -1930,7 +1930,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 	 * just abort and return filer policy value to not use the
 	 * accept list.
 	 */
-	list_for_each_entry(params, &hdev->pend_le_conns, action) {
+	list_for_each_entry_safe(params, tmp, &hdev->pend_le_conns, action) {
 		err = hci_le_add_accept_list_sync(hdev, params, &num_entries);
 		if (err)
 			goto done;
@@ -1940,7 +1940,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
 	 * the list of pending reports and also add these to the
 	 * accept list if there is still space. Abort if space runs out.
 	 */
-	list_for_each_entry(params, &hdev->pend_le_reports, action) {
+	list_for_each_entry_safe(params, tmp, &hdev->pend_le_reports, action) {
 		err = hci_le_add_accept_list_sync(hdev, params, &num_entries);
 		if (err)
 			goto done;
-- 
2.37.1.359.gd136c6c3e2-goog