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

[syzbot <syzbot+740e04c2a93467a0f8c8@syzkaller.appspotmail.com>]

> On 18/02/2026 09:30, 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")
>
> Reported-by: syzbot+740e04c2a93467a0f8c8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=740e04c2a93467a0f8c8
>
> #syz unfix
> #syz test
>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
>> ---
>> v6:
>> - use ssize_t for data_len parameter to guard against underflows
>> - omit unneeded data_len decrements at the end of the functions
>> 
>> v5:
>> - also check helper functions in nci_extract_rf_params_nfcf_passive_listen
>>   and nci_rf_discover_ntf_packet
>> 
>> v4:
>> - formatting fixes
>> 
>> v3:
>> - perform complete checks
>> - replace magic numbers with offsetofend and sizeof
>> 
>> v2:
>> - Reference correct commit hash
>> 
>> ---
>>  net/nfc/nci/ntf.c | 159 ++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 141 insertions(+), 18 deletions(-)
>> 
>> diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
>> index 418b84e2b260..c96512bb8653 100644
>> --- a/net/nfc/nci/ntf.c
>> +++ b/net/nfc/nci/ntf.c
>> @@ -58,7 +58,7 @@ 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))
>> +	if (skb->len < offsetofend(struct nci_core_conn_credit_ntf, num_entries))
>>  		return -EINVAL;
>>  
>>  	ntf = (struct nci_core_conn_credit_ntf *)skb->data;
>> @@ -68,6 +68,10 @@ static int nci_core_conn_credits_ntf_packet(struct nci_dev *ndev,
>>  	if (ntf->num_entries > NCI_MAX_NUM_CONN)
>>  		ntf->num_entries = NCI_MAX_NUM_CONN;
>>  
>> +	if (skb->len < offsetofend(struct nci_core_conn_credit_ntf, num_entries) +
>> +			ntf->num_entries * sizeof(struct conn_credit_entry))
>> +		return -EINVAL;
>> +
>>  	/* update the credits */
>>  	for (i = 0; i < ntf->num_entries; i++) {
>>  		ntf->conn_entries[i].conn_id =
>> @@ -138,23 +142,48 @@ 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, ssize_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++;
>> +	}
>>  
>>  	pr_debug("sel_res_len %d, sel_res 0x%x\n",
>>  		 nfca_poll->sel_res_len,
>> @@ -166,12 +195,21 @@ nci_extract_rf_params_nfca_passive_poll(struct nci_dev *ndev,
>>  static const __u8 *
>>  nci_extract_rf_params_nfcb_passive_poll(struct nci_dev *ndev,
>>  					struct rf_tech_specific_params_nfcb_poll *nfcb_poll,
>> -					const __u8 *data)
>> +					const __u8 *data, ssize_t data_len)
>>  {
>> +	/* Check if we have enough data for sensb_res_len (1 byte) */
>> +	if (data_len < 1)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	nfcb_poll->sensb_res_len = min_t(__u8, *data++, NFC_SENSB_RES_MAXSIZE);
>> +	data_len--;
>>  
>>  	pr_debug("sensb_res_len %d\n", nfcb_poll->sensb_res_len);
>>  
>> +	/* Check if we have enough data for sensb_res */
>> +	if (data_len < nfcb_poll->sensb_res_len)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	memcpy(nfcb_poll->sensb_res, data, nfcb_poll->sensb_res_len);
>>  	data += nfcb_poll->sensb_res_len;
>>  
>> @@ -181,14 +219,29 @@ 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, ssize_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;
>>  
>> @@ -198,22 +251,49 @@ nci_extract_rf_params_nfcf_passive_poll(struct nci_dev *ndev,
>>  static const __u8 *
>>  nci_extract_rf_params_nfcv_passive_poll(struct nci_dev *ndev,
>>  					struct rf_tech_specific_params_nfcv_poll *nfcv_poll,
>> -					const __u8 *data)
>> +					const __u8 *data, ssize_t data_len)
>>  {
>> +	/* Skip 1 byte (reserved) */
>> +	if (data_len < 1)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	++data;
>> +	data_len--;
>> +
>> +	/* Check if we have enough data for dsfid (1 byte) */
>> +	if (data_len < 1)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	nfcv_poll->dsfid = *data++;
>> +	data_len--;
>> +
>> +	/* Check if we have enough data for uid (8 bytes) */
>> +	if (data_len < NFC_ISO15693_UID_MAXSIZE)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	memcpy(nfcv_poll->uid, data, NFC_ISO15693_UID_MAXSIZE);
>>  	data += NFC_ISO15693_UID_MAXSIZE;
>> +
>>  	return data;
>>  }
>>  
>>  static const __u8 *
>>  nci_extract_rf_params_nfcf_passive_listen(struct nci_dev *ndev,
>>  					  struct rf_tech_specific_params_nfcf_listen *nfcf_listen,
>> -					  const __u8 *data)
>> +					  const __u8 *data, ssize_t data_len)
>>  {
>> +	/* Check if we have enough data for local_nfcid2_len (1 byte) */
>> +	if (data_len < 1)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	nfcf_listen->local_nfcid2_len = min_t(__u8, *data++,
>>  					      NFC_NFCID2_MAXSIZE);
>> +	data_len--;
>> +
>> +	/* Check if we have enough data for local_nfcid2 */
>> +	if (data_len < nfcf_listen->local_nfcid2_len)
>> +		return ERR_PTR(-EINVAL);
>> +
>>  	memcpy(nfcf_listen->local_nfcid2, data, nfcf_listen->local_nfcid2_len);
>>  	data += nfcf_listen->local_nfcid2_len;
>>  
>> @@ -364,7 +444,7 @@ 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))
>> +	if (skb->len < offsetofend(struct nci_rf_discover_ntf, rf_tech_specific_params_len))
>>  		return -EINVAL;
>>  
>>  	data = skb->data;
>> @@ -380,26 +460,42 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
>>  	pr_debug("rf_tech_specific_params_len %d\n",
>>  		 ntf.rf_tech_specific_params_len);
>>  
>> +	if (skb->len < (data - skb->data) +
>> +			ntf.rf_tech_specific_params_len + sizeof(ntf.ntf_type))
>> +		return -EINVAL;
>> +
>>  	if (ntf.rf_tech_specific_params_len > 0) {
>>  		switch (ntf.rf_tech_and_mode) {
>>  		case NCI_NFC_A_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfca_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfca_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfca_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return PTR_ERR(data);
>>  			break;
>>  
>>  		case NCI_NFC_B_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcb_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcb_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcb_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return PTR_ERR(data);
>>  			break;
>>  
>>  		case NCI_NFC_F_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcf_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcf_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcf_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return PTR_ERR(data);
>>  			break;
>>  
>>  		case NCI_NFC_V_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcv_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcv_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcv_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return PTR_ERR(data);
>>  			break;
>>  
>>  		default:
>> @@ -596,7 +692,7 @@ 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))
>> +	if (skb->len < offsetofend(struct nci_rf_intf_activated_ntf, rf_tech_specific_params_len))
>>  		return -EINVAL;
>>  
>>  	data = skb->data;
>> @@ -628,26 +724,41 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
>>  	if (ntf.rf_interface == NCI_RF_INTERFACE_NFCEE_DIRECT)
>>  		goto listen;
>>  
>> +	if (skb->len < (data - skb->data) + ntf.rf_tech_specific_params_len)
>> +		return -EINVAL;
>> +
>>  	if (ntf.rf_tech_specific_params_len > 0) {
>>  		switch (ntf.activation_rf_tech_and_mode) {
>>  		case NCI_NFC_A_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfca_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfca_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfca_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return -EINVAL;
>>  			break;
>>  
>>  		case NCI_NFC_B_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcb_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcb_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcb_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return -EINVAL;
>>  			break;
>>  
>>  		case NCI_NFC_F_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcf_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcf_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcf_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return -EINVAL;
>>  			break;
>>  
>>  		case NCI_NFC_V_PASSIVE_POLL_MODE:
>>  			data = nci_extract_rf_params_nfcv_passive_poll(ndev,
>> -				&(ntf.rf_tech_specific_params.nfcv_poll), data);
>> +				&(ntf.rf_tech_specific_params.nfcv_poll), data,
>> +				ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return -EINVAL;
>>  			break;
>>  
>>  		case NCI_NFC_A_PASSIVE_LISTEN_MODE:
>> @@ -657,7 +768,9 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
>>  		case NCI_NFC_F_PASSIVE_LISTEN_MODE:
>>  			data = nci_extract_rf_params_nfcf_passive_listen(ndev,
>>  				&(ntf.rf_tech_specific_params.nfcf_listen),
>> -				data);
>> +				data, ntf.rf_tech_specific_params_len);
>> +			if (IS_ERR(data))
>> +				return -EINVAL;
>>  			break;
>>  
>>  		default:
>> @@ -668,6 +781,13 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
>>  		}
>>  	}
>>  
>> +	if (skb->len < (data - skb->data) +
>> +			sizeof(ntf.data_exch_rf_tech_and_mode) +
>> +			sizeof(ntf.data_exch_tx_bit_rate) +
>> +			sizeof(ntf.data_exch_rx_bit_rate) +
>> +			sizeof(ntf.activation_params_len))
>> +		return -EINVAL;
>> +
>>  	ntf.data_exch_rf_tech_and_mode = *data++;
>>  	ntf.data_exch_tx_bit_rate = *data++;
>>  	ntf.data_exch_rx_bit_rate = *data++;
>> @@ -679,6 +799,9 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
>>  	pr_debug("data_exch_rx_bit_rate 0x%x\n", ntf.data_exch_rx_bit_rate);
>>  	pr_debug("activation_params_len %d\n", ntf.activation_params_len);
>>  
>> +	if (skb->len < (data - skb->data) + ntf.activation_params_len)
>> +		return -EINVAL;
>> +
>>  	if (ntf.activation_params_len > 0) {
>>  		switch (ntf.rf_interface) {
>>  		case NCI_RF_INTERFACE_ISO_DEP:
>
>
> Best regards,
> Krzysztof

Command #2:
This bug is already marked as fixed. No point in testing.