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 for Android: free password hash cracker in your pocket

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

Message-ID: <e9d63690-cdf4-41b7-98a1-6ebdf818fa11@0x83.eu>
Date: Sat, 7 Feb 2026 01:58:26 +0100
From: "Lukas K." <lu@...3.eu>
To: Paolo Abeni <pabeni@...hat.com>,
 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 15.01.26 12:25, Paolo Abeni wrote:
> 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);
>>   
As far as I can tell, the code ensures that data_len never underflows. 
If that'd happen all subsequent length checks would pass, even though 
they should not. Using  a signed type (ssize_t?) would provide some 
extra safety in here.
>> +	/* 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--;