Re: [PATCH net] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers

From: Simon Horman

Date: Thu Jun 25 2026 - 05:19:20 EST


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://netdev-ai.bots.linux.dev/sashiko/
---
nfc: nci: fix uninit-value in the RF discover/activated NTF handlers

This zero-initialises the on-stack nci_rf_discover_ntf and
nci_rf_intf_activated_ntf structs in the RF discover and interface
activated NTF handlers so that the technology- and activation-specific
unions read back as zero when the corresponding length fields are zero.

Link: https://lore.kernel.org/netdev/20260623172109.1105965-2-horms@xxxxxxxxxx/

> - activated: with rf_tech_specific_params_len == 0 the same union is read
> via nci_target_auto_activated(); with activation_params_len == 0 the
> activation_params union is read by nci_store_ats_nfc_iso_dep() into
> ndev->target_ats.

[Low]
Does the third scenario in this paragraph match the code?

Looking at nci_store_ats_nfc_iso_dep():

static int nci_store_ats_nfc_iso_dep(struct nci_dev *ndev,
const struct nci_rf_intf_activated_ntf *ntf)
{
ndev->target_ats_len = 0;

if (ntf->activation_params_len <= 0)
return NCI_STATUS_OK;

if (ntf->activation_params.nfca_poll_iso_dep.rats_res_len > NFC_ATS_MAXSIZE) {
...

The function returns NCI_STATUS_OK before any access to the
activation_params union when activation_params_len is 0, so the union
is not read in that case.

The sibling nci_store_general_bytes_nfc_dep() has the same early return
on activation_params_len <= 0.

The two rf_tech_specific_params_len == 0 cases listed earlier in the
commit message do match the code (nci_add_new_protocol() and
nci_target_auto_activated() unconditionally pass
&ntf.rf_tech_specific_params even when rf_tech_specific_params_len is
0), and the KMSAN trace quoted in the commit message only mentions the
nci_add_new_protocol() path.

The ntf = {} change still looks correct and necessary for the
rf_tech_specific_params union. Could the third bullet be dropped or
adjusted so the commit message reflects only the paths that actually
read uninitialised bytes?