[PATCH 0/1] nfc: nci: do not process unexpected or invalid CORE_CONN_CREATE_RSP

[PATCH 0/1] nfc: nci: do not process unexpected or invalid CORE_CONN_CREATE_RSP

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

Hello,

This patch addresses several issues in the NCI core logical connection
handling where an unsolicited or malformed CORE_CONN_CREATE_RSP packet
can corrupt connection tracking, shadow the static RF connection, and
cause TX queue stalls or hung waiters.

Problem Overview:
=================
In the NCI core stack, logical connections can be created by sending
NCI_OP_CORE_CONN_CREATE_CMD to the NFCC, which answers with
NCI_OP_CORE_CONN_CREATE_RSP. nci_core_conn_create_rsp_packet() parses
this response and adds a new struct nci_conn_info to ndev->conn_info_list.

However, nci_core_conn_create_rsp_packet() had several vulnerabilities:
1. No Pending Command Check:
   It did not check whether a connection creation command was actually
   pending. An unsolicited or delayed CORE_CONN_CREATE_RSP packet
   unconditionally allocated and inserted a connection into
   ndev->conn_info_list, and called nci_req_complete(ndev, status),
   prematurely completing whatever unrelated request was in-flight.
2. Connection ID Collision & Shadowing:
   Dynamic logical connections allocated by the NFCC must not use
   NCI_STATIC_RF_CONN_ID (0x00) or collide with existing connections.
   Furthermore, nci_core_conn_create_rsp_packet() used list_add() to
   prepend the new connection to ndev->conn_info_list. Because
   connection lookups (e.g. in nci_tx_work() and
   nci_data_exchange_complete()) use first-match semantics, prepending
   allowed an injected connection with conn_id = 0 to shadow
   ndev->rf_conn_info. This caused:
   - Cross-connection credit accounting in nci_tx_work().
   - TX queue stall when credits == 0 (frame wedged without timer).
   - Dropped RX completion (cb == NULL on the rogue connection, hanging
     the real waiter registered by nci_transceive()).
3. Missing Length Validation:
   The handler accessed payload fields without checking whether
   skb->len >= sizeof(struct nci_core_conn_create_rsp), risking
   out-of-bounds reads.
4. Spurious HCI conn_info Assignment:
   ndev->hci_dev->conn_info was updated whenever cur_params.id matched
   hci_dev->nfcee_id. Because both default to 0, non-NFCEE connections
   (such as loopback) erroneously updated ndev->hci_dev->conn_info.

Solution:
=========
- Add an NCI_CONN_CREATE_PENDING flag in enum nci_flag to track
  in-flight connection creation commands, and reject unsolicited or
  delayed responses.
- Validate packet length before reading payload fields.
- Reject responses that allocate NCI_STATIC_RF_CONN_ID or duplicate
  existing connection IDs.
- Append new connections with list_add_tail() instead of list_add().
- Restrict ndev->hci_dev->conn_info assignment to NFCEE destination
  types.

Non-overlap with adjacent fixes:
================================
This issue does not overlap with the adjacent upstream fixes in this
area: Lin Ma's commit 1b1499a817c9 ("nfc: nci: fix the UAF of
rf_conn_info object") addresses a use-after-free in the conn_close
path, and Yun Zhou's commit d56575a2595e ("nfc: nci: fix use of
uninitialized memory in CORE_INIT_RSP parsing") addresses a different
packet. The problem fixed here is the list_add() head-insertion
combined with first-match lookup semantics (core.c:43-54), which lets
a forged connection shadow the real one.

Testing:
========
Verified with QEMU arm64 empirical test harness. Unsolicited
CORE_CONN_CREATE_RSP frames were rejected with a rate-limited warning;
conn_count remained 1, and the static RF connection was not shadowed.
Verified with module compilation and checkpatch.pl (0 errors, 0 warnings).

Thanks,
Yuchao Zhang

Yuchao Zhang (1):
  nfc: nci: do not process unexpected or invalid CORE_CONN_CREATE_RSP

 include/net/nfc/nci_core.h |  1 +
 net/nfc/nci/core.c         |  2 ++
 net/nfc/nci/rsp.c          | 37 ++++++++++++++++++++++++++++++-------
 3 files changed, 33 insertions(+), 7 deletions(-)

-- 
2.53.0

[PATCH 1/1] nfc: nci: do not process unexpected or invalid CORE_CONN_CREATE_RSP

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

When an NCI_OP_CORE_CONN_CREATE_RSP packet is received,
nci_core_conn_create_rsp_packet() parses it and creates a logical
connection entry in ndev->conn_info_list.

However, nci_core_conn_create_rsp_packet() suffers from several issues:
1. It does not verify that a connection creation command is currently
   pending. An unsolicited or delayed CORE_CONN_CREATE_RSP packet
   unconditionally creates a rogue connection and calls
   nci_req_complete(), prematurely completing unrelated in-flight
   requests.
2. It lacks bounds checking against sizeof(struct nci_core_conn_create_rsp)
   for the packet payload, leading to potential out-of-bounds reads.
3. It does not validate rsp->conn_id. Dynamic logical connections
   assigned by the NFCC must not use NCI_STATIC_RF_CONN_ID (0x00) or
   collide with any existing connection ID.
4. It inserts the new connection at the head of ndev->conn_info_list
   via list_add(). Because connection lookup (e.g. in nci_tx_work() and
   nci_data_exchange_complete()) uses first-match semantics, prepending
   allows a newly created connection with a duplicate ID to shadow the
   static RF connection or earlier connections.
   Furthermore, ndev->hci_dev->conn_info is mistakenly updated even
   when the destination type is not NCI_DESTINATION_NFCEE because
   cur_params.id and nfcee_id both default to 0.

Fix this by:
- Adding an NCI_CONN_CREATE_PENDING flag in enum nci_flag to track
  in-flight connection creation commands, and rejecting unsolicited or
  delayed responses.
- Adding packet length verification before reading payload fields.
- Rejecting responses that allocate NCI_STATIC_RF_CONN_ID or duplicate
  existing connection IDs.
- Appending new connections with list_add_tail() instead of list_add().
- Restricting ndev->hci_dev->conn_info assignment to NFCEE destination
  types.

Fixes: 736bb9577407 ("NFC: nci: Support logical connections management")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 include/net/nfc/nci_core.h |  1 +
 net/nfc/nci/core.c         |  2 ++
 net/nfc/nci/rsp.c          | 37 ++++++++++++++++++++++++++++++-------
 3 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 664d5058e66e..a4652ffc173d 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -31,6 +31,7 @@ enum nci_flag {
 	NCI_DATA_EXCHANGE,
 	NCI_DATA_EXCHANGE_TO,
 	NCI_UNREG,
+	NCI_CONN_CREATE_PENDING,
 };
 
 /* NCI device states */
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 5f46c4b5720f..b5fa17f0524e 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -134,6 +134,7 @@ static int __nci_request(struct nci_dev *ndev,
 	}
 
 	ndev->req_status = ndev->req_result = 0;
+	clear_bit(NCI_CONN_CREATE_PENDING, &ndev->flags);
 
 	return rc;
 }
@@ -709,6 +710,7 @@ static void nci_core_conn_create_req(struct nci_dev *ndev, const void *opt)
 {
 	const struct core_conn_create_data *data = opt;
 
+	set_bit(NCI_CONN_CREATE_PENDING, &ndev->flags);
 	nci_send_cmd(ndev, NCI_OP_CORE_CONN_CREATE_CMD, data->length, data->cmd);
 }
 
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index b0ab4f5acbce..a56a7392ab5e 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -305,15 +305,37 @@ static void nci_nfcee_mode_set_rsp_packet(struct nci_dev *ndev,
 static void nci_core_conn_create_rsp_packet(struct nci_dev *ndev,
 					    const struct sk_buff *skb)
 {
-	__u8 status = skb->data[0];
 	struct nci_conn_info *conn_info = NULL;
 	const struct nci_core_conn_create_rsp *rsp;
+	__u8 status;
+
+	if (skb->len < 1)
+		return;
 
+	status = skb->data[0];
 	pr_debug("status 0x%x\n", status);
 
+	if (!test_and_clear_bit(NCI_CONN_CREATE_PENDING, &ndev->flags)) {
+		pr_warn_ratelimited("unexpected CORE_CONN_CREATE_RSP\n");
+		return;
+	}
+
 	if (status == NCI_STATUS_OK) {
+		if (skb->len < sizeof(*rsp)) {
+			status = NCI_STATUS_SYNTAX_ERROR;
+			goto exit;
+		}
+
 		rsp = (struct nci_core_conn_create_rsp *)skb->data;
 
+		if (rsp->conn_id == NCI_STATIC_RF_CONN_ID ||
+		    nci_get_conn_info_by_conn_id(ndev, rsp->conn_id)) {
+			pr_warn_ratelimited("invalid or duplicate conn_id 0x%x\n",
+					    rsp->conn_id);
+			status = NCI_STATUS_REJECTED;
+			goto exit;
+		}
+
 		conn_info = devm_kzalloc(&ndev->nfc_dev->dev,
 					 sizeof(*conn_info), GFP_KERNEL);
 		if (!conn_info) {
@@ -339,21 +361,22 @@ static void nci_core_conn_create_rsp_packet(struct nci_dev *ndev,
 		 */
 
 		INIT_LIST_HEAD(&conn_info->list);
-		list_add(&conn_info->list, &ndev->conn_info_list);
+		list_add_tail(&conn_info->list, &ndev->conn_info_list);
 
-		if (ndev->cur_params.id == ndev->hci_dev->nfcee_id)
+		if (conn_info->dest_type == NCI_DESTINATION_NFCEE &&
+		    ndev->cur_params.id == ndev->hci_dev->nfcee_id)
 			ndev->hci_dev->conn_info = conn_info;
 
-		conn_info->conn_id = rsp->conn_id;
 		conn_info->max_pkt_payload_len = rsp->max_ctrl_pkt_payload_len;
 		atomic_set(&conn_info->credits_cnt, rsp->credits_cnt);
 	}
 
+	nci_req_complete(ndev, status);
+	return;
+
 free_conn_info:
-	if (status == NCI_STATUS_REJECTED)
-		devm_kfree(&ndev->nfc_dev->dev, conn_info);
+	devm_kfree(&ndev->nfc_dev->dev, conn_info);
 exit:
-
 	nci_req_complete(ndev, status);
 }
 
-- 
2.53.0