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

[Simon Horman]

On Fri, Sep 18, 2026 at 10:04:12AM +0800, zjamg wrote:
> From: Yuchao Zhang <ndaugoing@xxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Yuchao Zhang <ndaugoing@xxxxxxxxx>
> ---
>  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,
>  };


Hi,

This patch seems to use a similar mechanism to another patch that
you recently posted

- [PATCH 1/1] nfc: nci: ignore unexpected CORE_RESET_NTF
  https://lore.kernel.org/netdev/20260918013337.82214-2-ndaugoing@xxxxxxxxx/

I forwarded an AI-generated review in response to that patch
and I am concerned that this patch has similar issues centred around:

* Use of a single-bit gate, in this case NCI_CONN_CREATE_PENDING, may
  match against a late-arriving packet for an earlier request
* Concurrency between a work queue handler and other execution paths


As this patch seems related to the one at the link above I would suggest
either concluding one first.  Or provided any updated versions in the form
of a patch-set that includes both patches.  And I would suggest the first
of those options would lead to a faster conclusion.

...