All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuchao Zhang <ndaugoing@gmail.com>
To: Simon Horman <horms@kernel.org>
Cc: David Heidelberg <david@ixit.cz>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	oe-linux-nfc@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Yuchao Zhang <ndaugoing@gmail.com>
Subject: [PATCH v3] nfc: nci: ignore unexpected CORE_RESET_NTF and CORE_RESET_RSP
Date: Sun, 27 Sep 2026 21:29:14 +0800	[thread overview]
Message-ID: <20260927132914.24648-1-ndaugoing@gmail.com> (raw)

Commit bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
added handling of CORE_RESET_NTF in nci_core_reset_ntf_packet(). When
received, it updates ndev->nci_ver, ndev->manufact_id, and
ndev->manufact_specific_info, and calls nci_req_complete(ndev,
NCI_STATUS_OK) to finish the pending reset request.

However, unlike other notification handlers in ntf.c (which validate
ndev->state before completing requests), nci_core_reset_ntf_packet()
does not check whether a core reset request is actually pending.
If an unsolicited or delayed CORE_RESET_NTF arrives (e.g. after a reset
command times out or from a misbehaving NFCC), it unconditionally:
1. Completes whatever request is currently in-flight (such as CORE_INIT,
   RF_DISCOVER, or CONN_CREATE) with NCI_STATUS_OK, leading to kernel
   state desynchronization.
2. Overwrites ndev->nci_ver and manufacturer info. Because
   ndev->nci_ver is used as a selector for subsequent packet formats
   and parsers (e.g., in nci_open_device() and nci_core_init_rsp_packet()),
   unexpectedly modifying it can cause protocol format confusion.

A similar issue exists in nci_core_reset_rsp_packet(): an unexpected or
delayed response packet can prematurely complete an unrelated in-flight
request.

Fix this by ensuring CORE_RESET_NTF and CORE_RESET_RSP are only processed
by the core layer when a reset command is actively awaiting them:
- Set NCI_RESET_PENDING in nci_reset_req() when sending CORE_RESET_CMD.
- In nci_core_reset_rsp_packet(), ignore the response if NCI_RESET_PENDING
  is not set. If set, atomically clear the flag and complete the request
  on failure or for NCI 1.x (checking skb->len >= sizeof(*rsp)).
- In __nci_request(), ensure NCI_RESET_PENDING is cleared upon request
  completion, cancellation, or timeout.
- In nci_core_reset_ntf_packet(), fail any in-flight reset request if the
  controller reports an unrecoverable error (0x00), and only process
  command-triggered notifications (0x02). Non-command triggers (e.g.
  power-on 0x01 or proprietary triggers) and unexpected notifications
  return 0 without modifying core state, ensuring driver-specific
  notification handlers (such as fdp firmware patch handling) still
  receive them.
- In nci_core_reset_ntf_packet(), support notifications shorter than 9
  bytes (e.g. NCI 2.0 frames with manufacturer_specific_len == 0),
  validating the fixed header length and safely bounding
  manufact_specific_info reads.

Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
v3:
 - In nci_core_reset_ntf_packet(), do not reject notifications shorter than
   9 bytes with -EINVAL; instead allow valid short frames (e.g. 2-byte NCI 1.x
   or 5-byte NCI 2.0 with manufacturer_specific_len == 0), safely cap field
   reads, and return 0 so driver-registered hooks (such as fdp firmware
   patching) are not skipped (per Sashiko AI).
 - Fix trigger semantics: in NCI, CORE_RESET_CMD trigger is 0x02
   (NCI_RESET_TRIGGER_RESET_CMD). If controller reports an unrecoverable error
   (0x00, NCI_RESET_TRIGGER_ERROR) while reset is pending, fail the request
   instead of completing with OK; ignore power-on (0x01) and proprietary
   triggers at core layer (per Sashiko AI).
 - In nci_core_reset_rsp_packet(), use test_and_clear_bit(NCI_RESET_PENDING)
   upon completing the request to consume the flag atomically (per Sashiko AI).
 - Link to v2: https://lore.kernel.org/netdev/20260922102144.39689-1-ndaugoing@gmail.com/

v2:
 - Do not abort the notification pipeline on unexpected CORE_RESET_NTF (return 0
   instead of -EINVAL), preserving driver-level hooks (e.g. fdp firmware
   patching) per Simon Horman.
 - Skip power-on notifications (trigger 0x01, identical in NCI 1.0 and 2.0)
   while gating both NCI 1.0 (0x00) and NCI 2.0 (0x02) command-triggered resets
   on NCI_RESET_PENDING alone.
 - Check NCI_RESET_PENDING in nci_core_reset_rsp_packet() to avoid completing
   unrelated requests on unexpected responses.
 - Explicitly check skb->len >= sizeof(*rsp) in nci_core_reset_rsp_packet()
   for NCI 1.x handling.
 - Link to v1: https://lore.kernel.org/netdev/20260918013337.82214-1-ndaugoing@gmail.com/

 include/net/nfc/nci.h      |  5 ++++
 include/net/nfc/nci_core.h |  1 +
 net/nfc/nci/core.c         |  3 +++
 net/nfc/nci/ntf.c          | 48 +++++++++++++++++++++++++++++++++-----
 net/nfc/nci/rsp.c          | 30 ++++++++++++++++++------
 5 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/include/net/nfc/nci.h b/include/net/nfc/nci.h
index 09efcaed7c3f..2ac45adba088 100644
--- a/include/net/nfc/nci.h
+++ b/include/net/nfc/nci.h
@@ -140,6 +140,11 @@
 #define NCI_RESET_TYPE_KEEP_CONFIG				0x00
 #define NCI_RESET_TYPE_RESET_CONFIG				0x01
 
+/* NCI Reset Triggers */
+#define NCI_RESET_TRIGGER_ERROR					0x00
+#define NCI_RESET_TRIGGER_POWER_ON				0x01
+#define NCI_RESET_TRIGGER_RESET_CMD				0x02
+
 /* NCI Static RF connection ID */
 #define NCI_STATIC_RF_CONN_ID					0x00
 
diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 664d5058e66e..504ea91ddf27 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_RESET_PENDING,
 };
 
 /* NCI device states */
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 5f46c4b5720f..5a90591cd9af 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_RESET_PENDING, &ndev->flags);
 
 	return rc;
 }
@@ -163,6 +164,8 @@ static void nci_reset_req(struct nci_dev *ndev, const void *opt)
 {
 	struct nci_core_reset_cmd cmd;
 
+	set_bit(NCI_RESET_PENDING, &ndev->flags);
+
 	cmd.reset_type = NCI_RESET_TYPE_RESET_CONFIG;
 	nci_send_cmd(ndev, NCI_OP_CORE_RESET_CMD, 1, &cmd);
 }
diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index f5c9a8ab7ec1..81ba6011c027 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -30,21 +30,57 @@
 static int nci_core_reset_ntf_packet(struct nci_dev *ndev,
 				     const struct sk_buff *skb)
 {
-	/* Handle NCI 2.x core reset notification */
 	const struct nci_core_reset_ntf *ntf;
 
-	if (skb->len < sizeof(struct nci_core_reset_ntf))
-		return -EINVAL;
+	/*
+	 * Return 0 on short or unhandled packets so driver-specific
+	 * notification hooks (such as fdp firmware patching) can still run.
+	 */
+	if (skb->len < 2)
+		return 0;
 
-	ntf = (struct nci_core_reset_ntf *)skb->data;
+	ntf = (const struct nci_core_reset_ntf *)skb->data;
+
+	/*
+	 * If the controller reports an unrecoverable error while a reset is
+	 * pending, fail the in-flight reset request.
+	 */
+	if (ntf->reset_trigger == NCI_RESET_TRIGGER_ERROR) {
+		if (test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags))
+			nci_req_complete(ndev, NCI_STATUS_FAILED);
+		return 0;
+	}
+
+	/*
+	 * Only CORE_RESET_CMD-triggered notifications (0x02) complete a
+	 * pending reset request in NCI 2.0. Power-on (0x01) and proprietary
+	 * triggers (>= 0xA0) must not update core protocol state.
+	 */
+	if (ntf->reset_trigger != NCI_RESET_TRIGGER_RESET_CMD)
+		return 0;
+
+	if (!test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags)) {
+		pr_warn_ratelimited("unexpected CORE_RESET_NTF\n");
+		return 0;
+	}
+
+	if (skb->len < offsetofend(struct nci_core_reset_ntf,
+				   manufacturer_specific_len)) {
+		nci_req_complete(ndev, NCI_STATUS_FAILED);
+		return 0;
+	}
 
 	ndev->nci_ver = ntf->nci_ver;
 	pr_debug("nci_ver 0x%x, config_status 0x%x\n",
 		 ntf->nci_ver, ntf->config_status);
 
 	ndev->manufact_id = ntf->manufact_id;
-	ndev->manufact_specific_info =
-		__le32_to_cpu(ntf->manufact_specific_info);
+	if (ntf->manufacturer_specific_len >= sizeof(__le32) &&
+	    skb->len >= sizeof(struct nci_core_reset_ntf))
+		ndev->manufact_specific_info =
+			__le32_to_cpu(ntf->manufact_specific_info);
+	else
+		ndev->manufact_specific_info = 0;
 
 	nci_req_complete(ndev, NCI_STATUS_OK);
 
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index b0ab4f5acbce..7a3c6bc1a0da 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -32,16 +32,32 @@ static void nci_core_reset_rsp_packet(struct nci_dev *ndev,
 
 	pr_debug("status 0x%x\n", rsp->status);
 
-	/* Handle NCI 1.x ver */
-	if (skb->len != 1) {
-		if (rsp->status == NCI_STATUS_OK) {
-			ndev->nci_ver = rsp->nci_ver;
-			pr_debug("nci_ver 0x%x, config_status 0x%x\n",
-				 rsp->nci_ver, rsp->config_status);
-		}
+	/*
+	 * If no reset request is pending, ignore unexpected responses to avoid
+	 * prematurely completing an unrelated request.
+	 */
+	if (!test_bit(NCI_RESET_PENDING, &ndev->flags))
+		return;
+
+	if (rsp->status != NCI_STATUS_OK) {
+		if (test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags))
+			nci_req_complete(ndev, rsp->status);
+		return;
+	}
+
+	/* Handle NCI 1.x ver: completed by RSP */
+	if (skb->len >= sizeof(*rsp)) {
+		if (!test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags))
+			return;
+
+		ndev->nci_ver = rsp->nci_ver;
+		pr_debug("nci_ver 0x%x, config_status 0x%x\n",
+			 rsp->nci_ver, rsp->config_status);
 
 		nci_req_complete(ndev, rsp->status);
 	}
+
+	/* NCI 2.x ver: status is OK, wait for CORE_RESET_NTF to complete */
 }
 
 static u8 nci_core_init_rsp_packet_v1(struct nci_dev *ndev,
-- 
2.53.0


             reply	other threads:[~2026-09-27 13:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 13:29 Yuchao Zhang [this message]
2026-10-01 10:43 ` [PATCH v3] nfc: nci: ignore unexpected CORE_RESET_NTF and CORE_RESET_RSP Simon Horman

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=20260927132914.24648-1-ndaugoing@gmail.com \
    --to=ndaugoing@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.