[PATCH 0/1] nfc: nci: ignore unexpected CORE_RESET_NTF

[PATCH 0/1] nfc: nci: ignore unexpected CORE_RESET_NTF

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

Hello,

This patch addresses an issue in the NCI core stack where an unexpected
or unsolicited CORE_RESET_NTF packet can prematurely complete unrelated
in-flight requests with NCI_STATUS_OK and corrupt protocol version state.

Problem Overview:
=================
Commit bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
added nci_core_reset_ntf_packet() to handle NCI 2.x CORE_RESET
notifications. When received, it updates ndev->nci_ver and manufacturer
information, and calls nci_req_complete(ndev, NCI_STATUS_OK).

Unlike other notification handlers in ntf.c (which validate ndev->state
before acting), nci_core_reset_ntf_packet() does not verify whether a
core reset request is actually pending.

If an unsolicited or delayed CORE_RESET_NTF is received:
1. If another request is currently in-flight (such as CORE_INIT,
   RF_DISCOVER, or CONN_CREATE), it prematurely completes that request
   with NCI_STATUS_OK, leading to state desynchronization.
2. Even when no request is in-flight, it unconditionally overwrites
   ndev->nci_ver and manufacturer info. Because ndev->nci_ver acts as
   a parser and packet format selector (e.g., in nci_open_device() and
   nci_core_init_rsp_packet()), unexpectedly modifying it can cause
   protocol format confusion.

Solution:
=========
Introduce an NCI_RESET_PENDING flag in enum nci_flag to ensure
CORE_RESET_NTF is only accepted while a reset command is actively
awaiting it.

Testing:
========
Verified with module compilation and checkpatch.pl (0 errors, 0 warnings).
Empirically confirmed that unsolicited CORE_RESET_NTF packets are safely
rejected with a warning while legitimate reset sequences continue to
complete normally.

Thanks,
Yuchao Zhang

Yuchao Zhang (1):
  nfc: nci: ignore unexpected CORE_RESET_NTF

 include/net/nfc/nci_core.h |  1 +
 net/nfc/nci/core.c         |  3 +++
 net/nfc/nci/ntf.c          |  5 +++++
 net/nfc/nci/rsp.c          | 15 ++++++++++-----
 4 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.53.0

[PATCH 1/1] nfc: nci: ignore unexpected CORE_RESET_NTF

[zjamg]

From: Yuchao Zhang <ndaugoing@gmail.com>

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.

Introduce an NCI_RESET_PENDING flag to ensure CORE_RESET_NTF is only
processed when a reset command is actively awaiting it:
- Set NCI_RESET_PENDING in nci_reset_req() when sending CORE_RESET_CMD.
- In nci_core_reset_rsp_packet(), if the reset failed or if handling
  NCI 1.x (where no notification is expected), clear NCI_RESET_PENDING
  and complete the request immediately.
- In __nci_request(), ensure NCI_RESET_PENDING is cleared upon request
  completion, cancellation, or timeout.
- In nci_core_reset_ntf_packet(), check and clear NCI_RESET_PENDING
  before updating device fields and completing the request. If the flag
  is not set, log a warning and drop the packet.

Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
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         |  3 +++
 net/nfc/nci/ntf.c          |  5 +++++
 net/nfc/nci/rsp.c          | 15 ++++++++++-----
 4 files changed, 19 insertions(+), 5 deletions(-)

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..69bc9047f057 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -36,6 +36,11 @@ static int nci_core_reset_ntf_packet(struct nci_dev *ndev,
 	if (skb->len < sizeof(struct nci_core_reset_ntf))
 		return -EINVAL;
 
+	if (!test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags)) {
+		pr_warn_ratelimited("unexpected CORE_RESET_NTF\n");
+		return -EINVAL;
+	}
+
 	ntf = (struct nci_core_reset_ntf *)skb->data;
 
 	ndev->nci_ver = ntf->nci_ver;
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index b0ab4f5acbce..a4bfe14372a8 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -32,14 +32,19 @@ static void nci_core_reset_rsp_packet(struct nci_dev *ndev,
 
 	pr_debug("status 0x%x\n", rsp->status);
 
+	if (rsp->status != NCI_STATUS_OK) {
+		clear_bit(NCI_RESET_PENDING, &ndev->flags);
+		nci_req_complete(ndev, rsp->status);
+		return;
+	}
+
 	/* 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);
-		}
+		ndev->nci_ver = rsp->nci_ver;
+		pr_debug("nci_ver 0x%x, config_status 0x%x\n",
+			 rsp->nci_ver, rsp->config_status);
 
+		clear_bit(NCI_RESET_PENDING, &ndev->flags);
 		nci_req_complete(ndev, rsp->status);
 	}
 }
-- 
2.53.0

Re: [PATCH 1/1] nfc: nci: ignore unexpected CORE_RESET_NTF

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918013337.82214-2-ndaugoing@gmail.com?part=1
---
nfc: nci: ignore unexpected CORE_RESET_NTF

This adds an NCI_RESET_PENDING flag that is set when CORE_RESET_CMD is sent
and cleared on request completion, so that CORE_RESET_NTF is only handled
while a reset request is outstanding. Unexpected notifications are dropped
with a ratelimited warning.

A few questions about the new gate below.

> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
> index 5f46c4b5720f6..5a90591cd9af7 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;
>  }

[Severity: Medium]
Since the flag carries no generation or sequence identity, can a
CORE_RESET_NTF belonging to an abandoned reset be matched against a later
reset attempt?

If reset attempt #1 times out here, the flag is cleared, but the
notification may already be sitting in nci_rx_work. The failure path in
nci_open_device() purges the queue but does not drain the worker:

	/* Init failed, cleanup */
	skb_queue_purge(&ndev->cmd_q);
	skb_queue_purge(&ndev->rx_q);
	skb_queue_purge(&ndev->tx_q);

	ndev->ops->close(ndev);

Only nci_close_device() flushes ndev->rx_wq, and it does that after dropping
req_lock; the exported nci_core_reset() path purges nothing.

If userspace then retries NFC_CMD_DEV_UP (or a driver calls
nci_core_reset()), nci_reset_req() sets the same bit again and the stale
frame is accepted by test_and_clear_bit() in nci_core_reset_ntf_packet(),
overwriting ndev->nci_ver / manufact_* from the old frame and completing
reset #2 with NCI_STATUS_OK before its own CORE_RESET_RSP has been seen.
The later real response can then complete the following CORE_INIT request.

> @@ -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);
>  }

[Severity: Medium]
Is a single bit enough synchronisation between the two contexts here?
nci_reset_req()/__nci_request() run in process context under
ndev->req_lock, while nci_core_reset_ntf_packet() runs from nci_rx_work on
ndev->rx_wq, and the two share no lock.

The check and the use are not atomic. After test_and_clear_bit() succeeds in
nci_core_reset_ntf_packet(), the worker can be preempted before it reaches:

	ndev->nci_ver = ntf->nci_ver;
	...
	nci_req_complete(ndev, NCI_STATUS_OK);

If the reset it belonged to times out in that window, __nci_request() zeroes
req_status and returns -ETIMEDOUT, and a new request can be armed
(req_status = NCI_REQ_PEND). nci_req_complete() only looks at the global
status:

	if (ndev->req_status == NCI_REQ_PEND) {
		ndev->req_result = result;
		ndev->req_status = NCI_REQ_DONE;
		complete(&ndev->req_completion);
	}

so the resumed worker completes the new request as successful, and writes
ndev->nci_ver while nci_open_device() may be reading it:

	if (ndev->nci_ver & NCI_VER_2_MASK)
		opt = &nci_init_v2_cmd;

Would tying the notification to a request generation, or serialising the
flag test with request start/teardown, close this?

> diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
> index f5c9a8ab7ec10..b7fa8980804da 100644
> --- a/net/nfc/nci/ntf.c
> +++ b/net/nfc/nci/ntf.c
> @@ -36,6 +36,11 @@ static int nci_core_reset_ntf_packet(struct nci_dev *ndev,
>  	if (skb->len < sizeof(struct nci_core_reset_ntf))
>  		return -EINVAL;
>  
> +	if (!test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags)) {
> +		pr_warn_ratelimited("unexpected CORE_RESET_NTF\n");
> +		return -EINVAL;
> +	}
> +

[Severity: High]
Does returning -EINVAL here break the fdp firmware update handshake?

In nci_ntf_packet() a non-zero return skips the driver dispatch:

	case NCI_OP_CORE_RESET_NTF:
		if (nci_core_reset_ntf_packet(ndev, skb))
			goto end;
		break;
	...
	nci_core_ntf_packet(ndev, ntf_opcode, skb);
end:
	kfree_skb(skb);

nci_core_ntf_packet() is the only dispatcher for driver core_ops handlers,
and drivers/nfc/fdp registers one for exactly this notification:

	static const struct nci_driver_ops fdp_core_ops[] = {
		...
		{
			.opcode = NCI_OP_CORE_RESET_NTF,
			.ntf = fdp_nci_core_reset_ntf_packet,
		},
	};

That hook is what wakes the patch path:

	static int fdp_nci_core_reset_ntf_packet(struct nci_dev *ndev,
						  struct sk_buff *skb)
	{
		struct fdp_nci_info *info = nci_get_drvdata(ndev);

		info->setup_reset_ntf = 1;
		wake_up(&info->setup_wq);

The notification it waits for is unsolicited, as the comment in
fdp_nci_send_patch() describes:

	 * command with NCI_PATCH_TYPE_EOT parameter. The device will send a
	 * NFCC_PATCH_NTF packet and a NCI_OP_CORE_RESET_NTF packet.

At that point the last request was nci_prop_cmd(NCI_OP_PROP_PATCH_OID),
whose completion also clears NCI_RESET_PENDING in __nci_request(), so the
flag is clear and the notification is dropped. Would
fdp_nci_patch_otp()/fdp_nci_patch_ram() then block forever in

	wait_event_interruptible(info->setup_wq, info->setup_reset_ntf);

with no timeout? That wait runs inside ndev->ops->setup(), which
nci_open_device() calls while holding ndev->req_lock, so a stalled task
would also block nci_close_device()/nci_unregister_device() and keep the
request_firmware() buffers pinned because fdp_nci_release_firmware() is
never reached.

Would it be enough to skip only the core-level bookkeeping (drop the state
update and break) instead of aborting the whole notification pipeline?

[Severity: Medium]
Should the decision use ntf->reset_trigger rather than a host-side flag?

The notification carries the discriminator itself:

	struct nci_core_reset_ntf {
		u8	reset_trigger;
		u8	config_status;
		...

but the accept/reject decision is taken before the packet is even parsed,
and reset_trigger is never read in the handler.

That cuts both ways. A genuine NFCC-initiated reset arriving outside a reset
request is discarded with only a warning, even though the controller has
returned to its initial state (RF discovery stopped, logical connections and
credits destroyed, routing table cleared), and the driver hook does not run
either.

Conversely, a spontaneous notification (reset_trigger not indicating
CORE_RESET_CMD) that happens to land inside the reset window is still
accepted, still overwrites ndev->nci_ver / manufact_id /
manufact_specific_info, and still calls:

	nci_req_complete(ndev, NCI_STATUS_OK);

Is the intended bug class actually eliminated, or only narrowed by timing?

> diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
> index b0ab4f5acbce8..a4bfe14372a81 100644
> --- a/net/nfc/nci/rsp.c
> +++ b/net/nfc/nci/rsp.c
> @@ -32,14 +32,19 @@ static void nci_core_reset_rsp_packet(struct nci_dev *ndev,
>  
>  	pr_debug("status 0x%x\n", rsp->status);
>  
> +	if (rsp->status != NCI_STATUS_OK) {
> +		clear_bit(NCI_RESET_PENDING, &ndev->flags);
> +		nci_req_complete(ndev, rsp->status);
> +		return;
> +	}
> +

[Severity: Medium]
Should the response handler test NCI_RESET_PENDING too, rather than only
clearing it?

nci_rsp_packet() dispatches purely by opcode with no correlation to the
outstanding command:

	switch (rsp_opcode) {
	case NCI_OP_CORE_RESET_RSP:
		nci_core_reset_rsp_packet(ndev, skb);
		break;

and nci_req_complete() only checks the global status, so a stray or delayed
CORE_RESET_RSP still completes whatever unrelated request is in flight
(CORE_INIT, RF_DISCOVER, CONN_CREATE) - the desynchronisation the commit
message sets out to remove.

This new block also widens that surface. Before the patch a 1-byte (NCI 2.x)
response completed nothing at all; now the status test runs before any length
discrimination, so a single 3-byte control frame (header plus a non-OK
status, accepted by nci_valid_size() since the declared payload is non-zero)
completes an unrelated request with a controller-chosen status.

There is also a mirror case: because the flag is cleared here without being
validated, a stray CORE_RESET_RSP during a genuine NCI 2.x reset disarms the
new guard, and the real CORE_RESET_NTF is then rejected as unexpected, so
the reset stalls until NCI_RESET_TIMEOUT.

>  	/* 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);
> -		}
> +		ndev->nci_ver = rsp->nci_ver;
> +		pr_debug("nci_ver 0x%x, config_status 0x%x\n",
> +			 rsp->nci_ver, rsp->config_status);
>  
> +		clear_bit(NCI_RESET_PENDING, &ndev->flags);
>  		nci_req_complete(ndev, rsp->status);
>  	}
>  }

[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but does
"skb->len != 1" read rsp->config_status past the received payload?

config_status is the third byte:

	struct nci_core_reset_rsp {
		__u8	status;
		__u8	nci_ver;
		__u8	config_status;
	} __packed;

and nci_valid_size() only requires the declared payload to be present and
non-zero:

	if (skb->len < hdr_size ||
	    skb->len < hdr_size + nci_plen(skb->data)) {
		return false;
	}

	if (!nci_plen(skb->data)) {

so a CORE_RESET_RSP with plen == 2 and status == NCI_STATUS_OK reaches this
block and the pr_debug() reads one byte beyond the payload. The patch keeps
the same condition (status == OK and len != 1), so this is unchanged
behaviour; would an explicit length check for the 3-byte form be worth
adding while this function is being touched?

Re: [PATCH 1/1] nfc: nci: ignore unexpected CORE_RESET_NTF

[Yuchao Zhang]

Hi Simon,

Thank you (and the sashiko reviewer) for the thorough review.
The points raised are completely valid and insightful.

1. Regarding FDP firmware update breakage (Severity: High):
You are absolutely right. In nci_ntf_packet(), returning -EINVAL from
nci_core_reset_ntf_packet() aborts the dispatch pipeline (goto end), which
prevents nci_core_ntf_packet() from running driver-specific handlers. In
drivers/nfc/fdp/fdp.c, the firmware patching path relies on this spontaneous
CORE_RESET_NTF to wake up info->setup_wq; dropping the packet causes it to
block indefinitely.

As suggested, in v2 we drop only the core-level bookkeeping (skipping the
ndev->nci_ver update and nci_req_complete) and return 0 so that driver-registered
ops continue to receive the notification.

2. Regarding ntf->reset_trigger (Severity: Medium):
The reset_trigger field is revision-dependent: in NCI 1.0, CORE_RESET_CMD
is reported as 0x00 (0x01 is powered-on), while NCI 2.0 defines 0x00 as
an unrecoverable error and reports CORE_RESET_CMD as 0x02. Hardcoding
either value would break the other generation of devices, so v2 gates
the core bookkeeping on NCI_RESET_PENDING alone and only skips power-on
notifications (trigger 0x01, which means the same in both revisions).
Power-on notifications never overlap a pending reset in the protocol
flow, and both revision's command-triggered values reach the flag gate.

3. Regarding nci_core_reset_rsp_packet() (Severity: Medium):
Agreed. nci_core_reset_rsp_packet() now checks whether NCI_RESET_PENDING
is set before calling nci_req_complete() on non-OK or NCI 1.x responses to avoid
prematurely completing an unrelated request.

4. Regarding NCI 1.x response length check (Severity: Low):
In v2, we explicitly check skb->len >= sizeof(*rsp) to ensure config_status is
not read out-of-bounds in pr_debug().

5. Regarding request serialization / stale notifications (Severity: Medium):
All requests are serialized under ndev->req_lock, and __nci_request() unconditionally
clears NCI_RESET_PENDING before dropping req_lock. Filtering out power-on notifications
(trigger 0x01) ensures spontaneous controller power-ups are ignored, while checking and
clearing the pending flag ensures each command matches at most one notification.

6. Regarding request generation (Severity: Medium on both questions):
The NCI protocol gives notifications no request correlation, so a stale
CORE_RESET_NTF/RSP that is already inside nci_rx_work when a reset times
out and userspace retries cannot be distinguished from the new attempt's
own frame: the flag gate and the trigger check narrow the window, but a
generation identity would require protocol support that does not exist.
The failure path purges queued frames; only an in-flight worker frame can
straddle, and the data it carries originates from the same controller.
If you would prefer, I can add a per-request generation check for the RSP
path where an identity exists, as a follow-up.

v2 patch has been sent as reply to this thread.

Thanks,
Yuchao

[PATCH v2] nfc: nci: ignore unexpected CORE_RESET_NTF and CORE_RESET_RSP

[Yuchao Zhang]

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, 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(), skip power-on notifications (trigger
  0x01, identical in NCI 1.0 and 2.0), then check and clear
  NCI_RESET_PENDING before updating device fields and completing the
  request. The CORE_RESET_CMD trigger value is revision-dependent
  (0x00 in NCI 1.0, 0x02 in NCI 2.0), so both are accepted and gated
  on NCI_RESET_PENDING alone. If unexpected, log a warning and return
  0 so driver-specific handlers (such as fdp firmware patch handling)
  still receive the notification.

Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <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.

 include/net/nfc/nci.h      |  5 +++++
 include/net/nfc/nci_core.h |  1 +
 net/nfc/nci/core.c         |  3 +++
 net/nfc/nci/ntf.c          | 17 ++++++++++++++++-
 net/nfc/nci/rsp.c          | 24 ++++++++++++++++++------
 5 files changed, 43 insertions(+), 7 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..57f99bf46a22 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -36,7 +36,22 @@ static int nci_core_reset_ntf_packet(struct nci_dev *ndev,
 	if (skb->len < sizeof(struct nci_core_reset_ntf))
 		return -EINVAL;
 
-	ntf = (struct nci_core_reset_ntf *)skb->data;
+	ntf = (const struct nci_core_reset_ntf *)skb->data;
+
+	/*
+	 * A power-on notification must not update core protocol state nor
+	 * complete a pending request.  The CORE_RESET_CMD trigger value is
+	 * revision-dependent (0x00 in NCI 1.0, 0x02 in NCI 2.0), so both
+	 * are let through to the reset-pending gate below instead.
+	 * Return 0 so driver-specific notification hooks can still run.
+	 */
+	if (ntf->reset_trigger == NCI_RESET_TRIGGER_POWER_ON)
+		return 0;
+
+	if (!test_and_clear_bit(NCI_RESET_PENDING, &ndev->flags)) {
+		pr_warn_ratelimited("unexpected CORE_RESET_NTF\n");
+		return 0;
+	}
 
 	ndev->nci_ver = ntf->nci_ver;
 	pr_debug("nci_ver 0x%x, config_status 0x%x\n",
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index b0ab4f5acbce..3a2491497e4b 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -32,14 +32,26 @@ static void nci_core_reset_rsp_packet(struct nci_dev *ndev,
 
 	pr_debug("status 0x%x\n", rsp->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) {
+		clear_bit(NCI_RESET_PENDING, &ndev->flags);
+		nci_req_complete(ndev, rsp->status);
+		return;
+	}
+
 	/* 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 (skb->len >= sizeof(*rsp)) {
+		ndev->nci_ver = rsp->nci_ver;
+		pr_debug("nci_ver 0x%x, config_status 0x%x\n",
+			 rsp->nci_ver, rsp->config_status);
 
+		clear_bit(NCI_RESET_PENDING, &ndev->flags);
 		nci_req_complete(ndev, rsp->status);
 	}
 }
-- 
2.53.0

Re: [PATCH v2] nfc: nci: ignore unexpected CORE_RESET_NTF and CORE_RESET_RSP

[David Heidelberg]

On 22/09/2026 10:07, Yuchao Zhang wrote:
> 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, 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(), skip power-on notifications (trigger
>    0x01, identical in NCI 1.0 and 2.0), then check and clear
>    NCI_RESET_PENDING before updating device fields and completing the
>    request. The CORE_RESET_CMD trigger value is revision-dependent
>    (0x00 in NCI 1.0, 0x02 in NCI 2.0), so both are accepted and gated
>    on NCI_RESET_PENDING alone. If unexpected, log a warning and return
>    0 so driver-specific handlers (such as fdp firmware patch handling)
>    still receive the notification.
> 
> Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yuchao Zhang <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.
Hello Yuchao,

please never send follow-ups as part of the thread. Always send them as a 
separate message (otherwise they may get lost and we already have patchwork wich 
can track the submissions.

Even better, try to use tools such as b4 to submit patches (easier for you, sent 
in expected format for us).

Thanks
David

Re: [PATCH v2] nfc: nci: ignore unexpected CORE_RESET_NTF and CORE_RESET_RSP

[Yuchao Zhang]

Hi David,

Thanks a lot for the pointer and explanation! I apologize for the
threading confusion — I mistakenly chained the v2 patch under the
previous review message using --in-reply-to.

I'll re-send the v2 patch as a clean, standalone thread right away so
that patchwork can track it properly, and I'll definitely adopt b4 for
future submissions.

Thanks again for keeping me on the right track!

Best regards,
Yuchao

David Heidelberg <david@ixit.cz> 于2026年9月22日周二 18:05写道：
>
> On 22/09/2026 10:07, Yuchao Zhang wrote:
> > 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, 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(), skip power-on notifications (trigger
> >    0x01, identical in NCI 1.0 and 2.0), then check and clear
> >    NCI_RESET_PENDING before updating device fields and completing the
> >    request. The CORE_RESET_CMD trigger value is revision-dependent
> >    (0x00 in NCI 1.0, 0x02 in NCI 2.0), so both are accepted and gated
> >    on NCI_RESET_PENDING alone. If unexpected, log a warning and return
> >    0 so driver-specific handlers (such as fdp firmware patch handling)
> >    still receive the notification.
> >
> > Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yuchao Zhang <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.
> Hello Yuchao,
>
> please never send follow-ups as part of the thread. Always send them as a
> separate message (otherwise they may get lost and we already have patchwork wich
> can track the submissions.
>
> Even better, try to use tools such as b4 to submit patches (easier for you, sent
> in expected format for us).
>
> Thanks
> David