mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: zjamg <ndaugoing@gmail.com>
To: David Heidelberg <david@ixit.cz>
Cc: Christophe Ricard <christophe.ricard@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	oe-linux-nfc@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Yuchao Zhang <ndaugoing@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH 1/1] nfc: nci: do not process unexpected or invalid CORE_CONN_CREATE_RSP
Date: Fri, 18 Sep 2026 10:04:12 +0800	[thread overview]
Message-ID: <20260918020412.82878-2-ndaugoing@gmail.com> (raw)
In-Reply-To: <20260918020412.82878-1-ndaugoing@gmail.com>

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


      reply	other threads:[~2026-09-18  2:04 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18  2:04 [PATCH 0/1] " zjamg
2026-09-18  2:04 ` zjamg [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260918020412.82878-2-ndaugoing@gmail.com \
    --to=ndaugoing@gmail.com \
    --cc=christophe.ricard@gmail.com \
    --cc=davem@davemloft.net \
    --cc=david@ixit.cz \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oe-linux-nfc@lists.linux.dev \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®