netdev - Re: [PATCH net v5] net: nfc: nci: Fix parameter validation for packet data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <21e77ec4-fa57-4a9f-8d9b-c417fd908ac6@redhat.com>
Date: Thu, 15 Jan 2026 12:25:20 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Michael Thalmeier <michael.thalmeier@...e.at>,
 Deepak Sharma <deepak.sharma.472935@...il.com>,
 Krzysztof Kozlowski <krzk@...nel.org>,
 Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Simon Horman
 <horms@...nel.org>, Jakub Kicinski <kuba@...nel.org>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
 Michael Thalmeier <michael@...lmeier.at>, stable@...r.kernel.org
Subject: Re: [PATCH net v5] net: nfc: nci: Fix parameter validation for packet
 data

On 1/12/26 1:48 PM, 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).
> 
> Fixes: 9c328f54741b ("net: nfc: nci: Add parameter validation for packet data")
> Cc: stable@...r.kernel.org
> Signed-off-by: Michael Thalmeier <michael.thalmeier@...e.at>

AFAICS this patch is doing at least 2 separate things:

- what described above,
- adding the missing checkes in
nci_extract_rf_params_nfcf_passive_listen and nci_rf_discover_ntf_packet

the latter is completely not described above and should land in separate
patch; note that whatever follows the '---' separator will not enter the
changelog.

> @@ -138,23 +142,49 @@ static int nci_core_conn_intf_error_ntf_packet(struct nci_dev *ndev,
>  static const __u8 *
>  nci_extract_rf_params_nfca_passive_poll(struct nci_dev *ndev,
>  					struct rf_tech_specific_params_nfca_poll *nfca_poll,
> -					const __u8 *data)
> +					const __u8 *data, size_t data_len)
>  {
> +	/* Check if we have enough data for sens_res (2 bytes) */
> +	if (data_len < 2)
> +		return ERR_PTR(-EINVAL);
> +
>  	nfca_poll->sens_res = __le16_to_cpu(*((__le16 *)data));
>  	data += 2;
> +	data_len -= 2;
> +
> +	/* Check if we have enough data for nfcid1_len (1 byte) */
> +	if (data_len < 1)
> +		return ERR_PTR(-EINVAL);
>  
>  	nfca_poll->nfcid1_len = min_t(__u8, *data++, NFC_NFCID1_MAXSIZE);
> +	data_len--;
>  
>  	pr_debug("sens_res 0x%x, nfcid1_len %d\n",
>  		 nfca_poll->sens_res, nfca_poll->nfcid1_len);
>  
> +	/* Check if we have enough data for nfcid1 */
> +	if (data_len < nfca_poll->nfcid1_len)
> +		return ERR_PTR(-EINVAL);
> +
>  	memcpy(nfca_poll->nfcid1, data, nfca_poll->nfcid1_len);
>  	data += nfca_poll->nfcid1_len;
> +	data_len -= nfca_poll->nfcid1_len;
> +
> +	/* Check if we have enough data for sel_res_len (1 byte) */
> +	if (data_len < 1)
> +		return ERR_PTR(-EINVAL);
>  
>  	nfca_poll->sel_res_len = *data++;
> +	data_len--;
> +
> +	if (nfca_poll->sel_res_len != 0) {
> +		/* Check if we have enough data for sel_res (1 byte) */
> +		if (data_len < 1)
> +			return ERR_PTR(-EINVAL);
>  
> -	if (nfca_poll->sel_res_len != 0)
>  		nfca_poll->sel_res = *data++;
> +		data_len--;

Last decrement not needed as data_len is never used afterwards.

> @@ -181,16 +221,32 @@ nci_extract_rf_params_nfcb_passive_poll(struct nci_dev *ndev,
>  static const __u8 *
>  nci_extract_rf_params_nfcf_passive_poll(struct nci_dev *ndev,
>  					struct rf_tech_specific_params_nfcf_poll *nfcf_poll,
> -					const __u8 *data)
> +					const __u8 *data, size_t data_len)
>  {
> +	/* Check if we have enough data for bit_rate (1 byte) */
> +	if (data_len < 1)
> +		return ERR_PTR(-EINVAL);
> +
>  	nfcf_poll->bit_rate = *data++;
> +	data_len--;
> +
> +	/* Check if we have enough data for sensf_res_len (1 byte) */
> +	if (data_len < 1)
> +		return ERR_PTR(-EINVAL);
> +
>  	nfcf_poll->sensf_res_len = min_t(__u8, *data++, NFC_SENSF_RES_MAXSIZE);
> +	data_len--;
>  
>  	pr_debug("bit_rate %d, sensf_res_len %d\n",
>  		 nfcf_poll->bit_rate, nfcf_poll->sensf_res_len);
>  
> +	/* Check if we have enough data for sensf_res */
> +	if (data_len < nfcf_poll->sensf_res_len)
> +		return ERR_PTR(-EINVAL);
> +
>  	memcpy(nfcf_poll->sensf_res, data, nfcf_poll->sensf_res_len);
>  	data += nfcf_poll->sensf_res_len;
> +	data_len -= nfcf_poll->sensf_res_len;

Same here.

/P