[PATCH] nfc: nci: fix use of uninitialized memory in CORE_INIT_RSP parsing

[PATCH] nfc: nci: fix use of uninitialized memory in CORE_INIT_RSP parsing

[Yun Zhou]

nci_core_init_rsp_packet_v1() and nci_core_init_rsp_packet_v2() parse
the CORE_INIT_RSP packet without validating that the skb contains
enough data. A malformed response (e.g. injected via virtual_ncidev)
can declare a large num_supported_rf_interfaces while providing
insufficient data, causing reads of uninitialized slab memory. This
is later used in nci_init_complete_req(), triggering a KMSAN
uninit-value warning.

Add skb length checks before accessing packet fields:
- Validate the skb can hold the fixed-size header before parsing.
- In v2, bounds-check each variable-length rf_interface entry and its
  extension parameters within the parsing loop.
- In v1, verify the skb is large enough for both the variable-length
  rf_interfaces array and the trailing rsp_2 structure.

Reported-by: syzbot+46ca2592193f2fb3debc@syzkaller.appspotmail.com
Fixes: bcd684aace34 ("net/nfc/nci: Support NCI 2.x initial sequence")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 net/nfc/nci/rsp.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index 9eeb862825c5..36e49fb7d894 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -50,6 +50,9 @@ static u8 nci_core_init_rsp_packet_v1(struct nci_dev *ndev,
 	const struct nci_core_init_rsp_1 *rsp_1 = (void *)skb->data;
 	const struct nci_core_init_rsp_2 *rsp_2;
 
+	if (skb->len < sizeof(*rsp_1))
+		return NCI_STATUS_SYNTAX_ERROR;
+
 	pr_debug("status 0x%x\n", rsp_1->status);
 
 	if (rsp_1->status != NCI_STATUS_OK)
@@ -62,6 +65,10 @@ static u8 nci_core_init_rsp_packet_v1(struct nci_dev *ndev,
 		min((int)ndev->num_supported_rf_interfaces,
 		    NCI_MAX_SUPPORTED_RF_INTERFACES);
 
+	if (skb->len < sizeof(*rsp_1) + rsp_1->num_supported_rf_interfaces +
+		       sizeof(*rsp_2))
+		return NCI_STATUS_SYNTAX_ERROR;
+
 	memcpy(ndev->supported_rf_interfaces,
 	       rsp_1->supported_rf_interfaces,
 	       ndev->num_supported_rf_interfaces);
@@ -87,15 +94,21 @@ static u8 nci_core_init_rsp_packet_v2(struct nci_dev *ndev,
 				      const struct sk_buff *skb)
 {
 	const struct nci_core_init_rsp_nci_ver2 *rsp = (void *)skb->data;
-	const u8 *supported_rf_interface = rsp->supported_rf_interfaces;
+	const u8 *supported_rf_interface;
+	const u8 *skb_end = skb->data + skb->len;
 	u8 rf_interface_idx = 0;
 	u8 rf_extension_cnt = 0;
 
+	if (skb->len < sizeof(*rsp))
+		return NCI_STATUS_SYNTAX_ERROR;
+
 	pr_debug("status %x\n", rsp->status);
 
 	if (rsp->status != NCI_STATUS_OK)
 		return rsp->status;
 
+	supported_rf_interface = rsp->supported_rf_interfaces;
+
 	ndev->nfcc_features = __le32_to_cpu(rsp->nfcc_features);
 	ndev->num_supported_rf_interfaces = rsp->num_supported_rf_interfaces;
 
@@ -104,13 +117,23 @@ static u8 nci_core_init_rsp_packet_v2(struct nci_dev *ndev,
 		    NCI_MAX_SUPPORTED_RF_INTERFACES);
 
 	while (rf_interface_idx < ndev->num_supported_rf_interfaces) {
-		ndev->supported_rf_interfaces[rf_interface_idx++] = *supported_rf_interface++;
+		/* Each entry: [rf_interface_type (1B)] [ext_count (1B)] [ext...] */
+		if (supported_rf_interface + 2 > skb_end)
+			break;
+		ndev->supported_rf_interfaces[rf_interface_idx] = *supported_rf_interface++;
 
 		/* skip rf extension parameters */
 		rf_extension_cnt = *supported_rf_interface++;
+		if (supported_rf_interface + rf_extension_cnt > skb_end)
+			break;
+
+		/* Only count the entry after full validation */
+		rf_interface_idx++;
 		supported_rf_interface += rf_extension_cnt;
 	}
 
+	ndev->num_supported_rf_interfaces = rf_interface_idx;
+
 	ndev->max_logical_connections = rsp->max_logical_connections;
 	ndev->max_routing_table_size =
 			__le16_to_cpu(rsp->max_routing_table_size);
-- 
2.43.0

Re: [PATCH] nfc: nci: fix use of uninitialized memory in CORE_INIT_RSP parsing

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

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://sashiko.dev
---
nfc: nci: fix use of uninitialized memory in CORE_INIT_RSP parsing

This commit adds skb length checks to nci_core_init_rsp_packet_v1() and
nci_core_init_rsp_packet_v2() before parsing the CORE_INIT_RSP packet to
prevent reads of uninitialized memory.

> diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
> index 9eeb862825c5f..36e49fb7d8941 100644
> --- a/net/nfc/nci/rsp.c
> +++ b/net/nfc/nci/rsp.c
> @@ -50,6 +50,9 @@ static u8 nci_core_init_rsp_packet_v1(struct nci_dev *ndev,
>  	const struct nci_core_init_rsp_1 *rsp_1 = (void *)skb->data;
>  	const struct nci_core_init_rsp_2 *rsp_2;
>  
> +	if (skb->len < sizeof(*rsp_1))
> +		return NCI_STATUS_SYNTAX_ERROR;
> +

Does this length check break the handling of valid NCI error responses?

If the device returns an error status, the response payload might only
contain the 1-byte status field. By requiring the full sizeof(*rsp_1)
before checking the status, will this return NCI_STATUS_SYNTAX_ERROR
instead of propagating the actual error code from the device?

>  	pr_debug("status 0x%x\n", rsp_1->status);
>  
>  	if (rsp_1->status != NCI_STATUS_OK)

[ ... ]

> @@ -87,15 +94,21 @@ static u8 nci_core_init_rsp_packet_v2(struct nci_dev *ndev,
>  				      const struct sk_buff *skb)
>  {
>  	const struct nci_core_init_rsp_nci_ver2 *rsp = (void *)skb->data;
> -	const u8 *supported_rf_interface = rsp->supported_rf_interfaces;
> +	const u8 *supported_rf_interface;
> +	const u8 *skb_end = skb->data + skb->len;
>  	u8 rf_interface_idx = 0;
>  	u8 rf_extension_cnt = 0;
>  
> +	if (skb->len < sizeof(*rsp))
> +		return NCI_STATUS_SYNTAX_ERROR;
> +

Could the same issue occur here for v2 responses?

If a command fails and the response payload only includes the status
field, checking for the full sizeof(*rsp) will drop the error response
and obscure the true failure reason.

>  	pr_debug("status %x\n", rsp->status);
>  
>  	if (rsp->status != NCI_STATUS_OK)
>  		return rsp->status;