[PATCH v2] net: nfc: nci: Fix parameter validation for packet data

[PATCH v2] net: nfc: nci: Fix parameter validation for packet data

[Michael Thalmeier]

Since commit 9c328f54741b ("net: nfc: nci: Add parameter validation for
packet data") communication with nci nfc chips is not working any more.

The mentioned commit tries to fix access of uninitialized data, but
failed to understand that in some cases the data packet is of variable
length and can therefore not be compared to the maximum packet length
given by the sizeof(struct).

For these cases it is only possible to check for minimum packet length.

Fixes: 9c328f54741b ("net: nfc: nci: Add parameter validation for packet data")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
---
Changes in v2:
- Reference correct commit hash

---
 net/nfc/nci/ntf.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index 418b84e2b260..5161e94f067f 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -58,7 +58,8 @@ static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
 	struct nci_conn_info *conn_info;
 	int i;
 
-	if (skb->len < sizeof(struct nci_core_conn_credit_ntf))
+	/* Minimal packet size for num_entries=1 is 1 x __u8 + 1 x conn_credit_entry */
+	if (skb->len < (sizeof(__u8) + sizeof(struct conn_credit_entry)))
 		return -EINVAL;
 
 	ntf = (struct nci_core_conn_credit_ntf *)skb->data;
@@ -364,7 +365,8 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
 	const __u8 *data;
 	bool add_target = true;
 
-	if (skb->len < sizeof(struct nci_rf_discover_ntf))
+	/* Minimal packet size is 5 if rf_tech_specific_params_len=0 */
+	if (skb->len < (5 * sizeof(__u8)))
 		return -EINVAL;
 
 	data = skb->data;
@@ -596,7 +598,10 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
 	const __u8 *data;
 	int err = NCI_STATUS_OK;
 
-	if (skb->len < sizeof(struct nci_rf_intf_activated_ntf))
+	/* Minimal packet size is 11 if
+	 * f_tech_specific_params_len=0 and activation_params_len=0
+	 */
+	if (skb->len < (11 * sizeof(__u8)))
 		return -EINVAL;
 
 	data = skb->data;
-- 
2.52.0

Re: [PATCH v2] net: nfc: nci: Fix parameter validation for packet data

[Paolo Abeni]

On 12/10/25 9:16 AM, Michael Thalmeier wrote:
> Since commit 9c328f54741b ("net: nfc: nci: Add parameter validation for
> packet data") communication with nci nfc chips is not working any more.
> 
> The mentioned commit tries to fix access of uninitialized data, but
> failed to understand that in some cases the data packet is of variable
> length and can therefore not be compared to the maximum packet length
> given by the sizeof(struct).
> 
> For these cases it is only possible to check for minimum packet length.
> 
> Fixes: 9c328f54741b ("net: nfc: nci: Add parameter validation for packet data")
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
> ---
> Changes in v2:
> - Reference correct commit hash

Minor nit: you should include the target tree ('net' in this case) in
the subj prefix.

>  net/nfc/nci/ntf.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
> index 418b84e2b260..5161e94f067f 100644
> --- a/net/nfc/nci/ntf.c
> +++ b/net/nfc/nci/ntf.c
> @@ -58,7 +58,8 @@ static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
>  	struct nci_conn_info *conn_info;
>  	int i;
>  
> -	if (skb->len < sizeof(struct nci_core_conn_credit_ntf))
> +	/* Minimal packet size for num_entries=1 is 1 x __u8 + 1 x conn_credit_entry */
> +	if (skb->len < (sizeof(__u8) + sizeof(struct conn_credit_entry)))
>  		return -EINVAL;

You can still perform a complete check, splitting such operation in two
steps:

First ensure that input contains enough data to include the length
related field; after reading such field check the the length is valid
and the packet len matches it.

>  
>  	ntf = (struct nci_core_conn_credit_ntf *)skb->data;
> @@ -364,7 +365,8 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
>  	const __u8 *data;
>  	bool add_target = true;
>  
> -	if (skb->len < sizeof(struct nci_rf_discover_ntf))
> +	/* Minimal packet size is 5 if rf_tech_specific_params_len=0 */
> +	if (skb->len < (5 * sizeof(__u8)))

Instead of using a magic number, you could/should use:
	 offsetof(struct nci_rf_discover_ntf, rf_tech_specific_params_len)

and will make the comment unneeded. Also the same consideration about
full validation apply here.

>  		return -EINVAL;
>  
>  	data = skb->data;
> @@ -596,7 +598,10 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
>  	const __u8 *data;
>  	int err = NCI_STATUS_OK;
>  
> -	if (skb->len < sizeof(struct nci_rf_intf_activated_ntf))
> +	/* Minimal packet size is 11 if
> +	 * f_tech_specific_params_len=0 and activation_params_len=0
> +	 */
> +	if (skb->len < (11 * sizeof(__u8)))
>  		return -EINVAL;

Again all the above applies here, too.

/P