Re: [PATCH net] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers
Simon Horman <[email protected]> Thu, 25 Jun 2026 10:18:29 +0100
| Newsgroups | dev.linux.lists.oe-linux-nfc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
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/[email protected]/ > - 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?