[PATCH] Bluetooth: MGMT: claim pending pairing command before completion

[Chengfeng Ye <nicoyip.dev@gmail.com>]

find_pairing() walks the pending command list without mgmt_pending_lock and
returns an unreferenced command.  Its callers then complete and remove the
command after the lookup.  cancel_pair_device() similarly drops the pending
lock after pending_find() and continues to use the returned command.

The HCI device lock does not close this gap because the SMP receive path
does not hold it.  The observed interleaving is:

  CPU 0 (cancel)                 CPU 1 (SMP failure)
  pending_find(cmd)
                                  find_pairing(cmd)
                                  cmd_complete(cmd)
                                  mgmt_pending_remove(cmd)
                                    kfree(cmd)
  cmd_complete(cmd)
  mgmt_pending_remove(cmd)

Thus CPU 0 dereferences and removes a command already freed by CPU 1.  The
same ownership race exists among the other pairing completion callbacks.

Find and unlink the matching pairing command while holding
mgmt_pending_lock, then complete and free the claimed command.  Use
mgmt_pending_valid() when pair_device() already has the exact command.
Keep cancellation limited to the first pending pairing command, as
before, so normal lookup and error behavior remain unchanged.

KASAN reported:

  BUG: KASAN: slab-use-after-free in mgmt_pending_remove+0x292/0x2f0
  Read of size 8 at addr ffff888100340498 by task poc/92

  Call Trace:
   mgmt_pending_remove+0x292/0x2f0
   cancel_pair_device+0x1d1/0x310
   hci_sock_sendmsg+0x1033/0x1ea0

  Allocated by task 86:
   mgmt_pending_new+0xb4/0x260
   mgmt_pending_add+0x1b/0x100
   pair_device+0x51d/0xcc0
   hci_sock_sendmsg+0x1033/0x1ea0

  Freed by task 87:
   kfree+0x131/0x3c0
   mgmt_pending_remove+0x20c/0x2f0
   mgmt_auth_failed+0x31a/0x4d0
   smp_recv_cb+0x4d6/0x8110
   l2cap_recv_frame+0xf14/0x9190
   l2cap_recv_acldata+0xa64/0xd40
   hci_rx_work+0x4ca/0x730

Fixes: f4a407bef20c ("Bluetooth: Wait for SMP key distribution completion when pairing")
Fixes: e1e930f591bf ("Bluetooth: Fix mgmt pairing failure when authentication fails")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/bluetooth/mgmt.c | 53 ++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1db10e0f617f..09cac79ce51b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -3514,21 +3514,34 @@ static int set_io_capability(struct sock *sk, struct hci_dev *hdev, void *data,
 				 NULL, 0);
 }
 
-static struct mgmt_pending_cmd *find_pairing(struct hci_conn *conn)
+static struct mgmt_pending_cmd *claim_pairing(struct hci_dev *hdev,
+					      struct hci_conn *conn,
+					      const bdaddr_t *bdaddr)
 {
-	struct hci_dev *hdev = conn->hdev;
 	struct mgmt_pending_cmd *cmd;
+	struct hci_conn *cmd_conn;
+
+	mutex_lock(&hdev->mgmt_pending_lock);
 
 	list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
 		if (cmd->opcode != MGMT_OP_PAIR_DEVICE)
 			continue;
 
-		if (cmd->user_data != conn)
+		cmd_conn = cmd->user_data;
+
+		if (conn && cmd_conn != conn)
 			continue;
 
+		if (bdaddr && (!cmd_conn || bacmp(bdaddr, &cmd_conn->dst)))
+			break;
+
+		list_del(&cmd->list);
+		mutex_unlock(&hdev->mgmt_pending_lock);
 		return cmd;
 	}
 
+	mutex_unlock(&hdev->mgmt_pending_lock);
+
 	return NULL;
 }
 
@@ -3566,10 +3579,10 @@ void mgmt_smp_complete(struct hci_conn *conn, bool complete)
 	u8 status = complete ? MGMT_STATUS_SUCCESS : MGMT_STATUS_FAILED;
 	struct mgmt_pending_cmd *cmd;
 
-	cmd = find_pairing(conn);
+	cmd = claim_pairing(conn->hdev, conn, NULL);
 	if (cmd) {
 		cmd->cmd_complete(cmd, status);
-		mgmt_pending_remove(cmd);
+		mgmt_pending_free(cmd);
 	}
 }
 
@@ -3579,14 +3592,14 @@ static void pairing_complete_cb(struct hci_conn *conn, u8 status)
 
 	BT_DBG("status %u", status);
 
-	cmd = find_pairing(conn);
+	cmd = claim_pairing(conn->hdev, conn, NULL);
 	if (!cmd) {
 		BT_DBG("Unable to find a pending command");
 		return;
 	}
 
 	cmd->cmd_complete(cmd, mgmt_status(status));
-	mgmt_pending_remove(cmd);
+	mgmt_pending_free(cmd);
 }
 
 static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
@@ -3598,14 +3611,14 @@ static void le_pairing_complete_cb(struct hci_conn *conn, u8 status)
 	if (!status)
 		return;
 
-	cmd = find_pairing(conn);
+	cmd = claim_pairing(conn->hdev, conn, NULL);
 	if (!cmd) {
 		BT_DBG("Unable to find a pending command");
 		return;
 	}
 
 	cmd->cmd_complete(cmd, mgmt_status(status));
-	mgmt_pending_remove(cmd);
+	mgmt_pending_free(cmd);
 }
 
 static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -3729,12 +3742,16 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 	}
 
 	conn->io_capability = cp->io_cap;
+
+	mutex_lock(&hdev->mgmt_pending_lock);
 	cmd->user_data = hci_conn_get(conn);
+	mutex_unlock(&hdev->mgmt_pending_lock);
 
 	if ((conn->state == BT_CONNECTED || conn->state == BT_CONFIG) &&
-	    hci_conn_security(conn, sec_level, auth_type, true)) {
+	    hci_conn_security(conn, sec_level, auth_type, true) &&
+	    mgmt_pending_valid(hdev, cmd)) {
 		cmd->cmd_complete(cmd, 0);
-		mgmt_pending_remove(cmd);
+		mgmt_pending_free(cmd);
 	}
 
 	err = 0;
@@ -3762,7 +3779,7 @@ static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 		goto unlock;
 	}
 
-	cmd = pending_find(MGMT_OP_PAIR_DEVICE, hdev);
+	cmd = claim_pairing(hdev, NULL, &addr->bdaddr);
 	if (!cmd) {
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_CANCEL_PAIR_DEVICE,
 				      MGMT_STATUS_INVALID_PARAMS);
@@ -3771,14 +3788,8 @@ static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	conn = cmd->user_data;
 
-	if (bacmp(&addr->bdaddr, &conn->dst) != 0) {
-		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_CANCEL_PAIR_DEVICE,
-				      MGMT_STATUS_INVALID_PARAMS);
-		goto unlock;
-	}
-
 	cmd->cmd_complete(cmd, MGMT_STATUS_CANCELLED);
-	mgmt_pending_remove(cmd);
+	mgmt_pending_free(cmd);
 
 	err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_CANCEL_PAIR_DEVICE, 0,
 				addr, sizeof(*addr));
@@ -10137,14 +10148,14 @@ void mgmt_auth_failed(struct hci_conn *conn, u8 hci_status)
 	ev.addr.type = link_to_bdaddr(conn->type, conn->dst_type);
 	ev.status = status;
 
-	cmd = find_pairing(conn);
+	cmd = claim_pairing(conn->hdev, conn, NULL);
 
 	mgmt_event(MGMT_EV_AUTH_FAILED, conn->hdev, &ev, sizeof(ev),
 		    cmd ? cmd->sk : NULL);
 
 	if (cmd) {
 		cmd->cmd_complete(cmd, status);
-		mgmt_pending_remove(cmd);
+		mgmt_pending_free(cmd);
 	}
 }
 
-- 
2.43.0