Netdev List
 help / color / mirror / Atom feed
* [PATCH net v2] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers
@ 2026-06-26  9:03 Samuel Page
  2026-06-27 18:41 ` David Heidelberg
  0 siblings, 1 reply; 2+ messages in thread
From: Samuel Page @ 2026-06-26  9:03 UTC (permalink / raw)
  To: David Heidelberg
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, oe-linux-nfc, netdev, linux-kernel, stable

nci_rf_discover_ntf_packet() and nci_rf_intf_activated_ntf_packet() each
parse a notification into an on-stack struct (nci_rf_discover_ntf /
nci_rf_intf_activated_ntf) that is not initialised. The RF
technology-specific parameters are only extracted when
rf_tech_specific_params_len is non-zero, so a notification that reports a
zero length leaves the rf_tech_specific_params union uninitialised - and
both handlers then pass it to nci_add_new_protocol(), which reads it:

 - discover:  nci_add_new_target() -> nci_add_new_protocol();
 - activated: nci_target_auto_activated() -> nci_add_new_protocol().

nci_add_new_protocol() uses nfca_poll->nfcid1_len as both a branch
condition and a memcpy() length and copies nfcid1/sens_res/sel_res into
ndev->targets, which is later exposed to user space via NFC_CMD_GET_TARGET.

  BUG: KMSAN: uninit-value in nci_add_new_protocol+0x624/0x6c0
   nci_add_new_protocol+0x624/0x6c0
   nci_ntf_packet+0x25b2/0x3c30
   nci_rx_work+0x318/0x5d0
   process_scheduled_works+0x84b/0x17a0
   worker_thread+0xc10/0x11b0
   kthread+0x376/0x500
  Local variable ntf.i created at:
   nci_ntf_packet+0xbc2/0x3c30

Zero-initialise both on-stack notifications so the union reads back as
zero when no technology-specific parameters are present.

Fixes: 019c4fbaa790 ("NFC: Add NCI multiple targets support")
Fixes: e8c0dacd9836 ("NFC: Update names and structs to NCI spec 1.0 d18")
Link: https://lore.kernel.org/netdev/20260623172109.1105965-2-horms@kernel.org/
Cc: stable@vger.kernel.org
Assisted-by: Bynario AI
Signed-off-by: Samuel Page <sam@bynar.io>
---
v2: Drop the inaccurate activation_params / NFC_ATTR_TARGET_ATS scenario
    from the commit message. No code change; the ntf = {} fix is unchanged.

 net/nfc/nci/ntf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index c96512bb8653..274d9a4202c9 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -440,7 +440,7 @@ void nci_clear_target_list(struct nci_dev *ndev)
 static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
 				      const struct sk_buff *skb)
 {
-	struct nci_rf_discover_ntf ntf;
+	struct nci_rf_discover_ntf ntf = {};
 	const __u8 *data;
 	bool add_target = true;
 
@@ -688,7 +688,7 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
 					    const struct sk_buff *skb)
 {
 	struct nci_conn_info *conn_info;
-	struct nci_rf_intf_activated_ntf ntf;
+	struct nci_rf_intf_activated_ntf ntf = {};
 	const __u8 *data;
 	int err = NCI_STATUS_OK;
 

base-commit: 02f144fbb4c86c360495d33debe307cb46a57f95
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v2] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers
  2026-06-26  9:03 [PATCH net v2] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers Samuel Page
@ 2026-06-27 18:41 ` David Heidelberg
  0 siblings, 0 replies; 2+ messages in thread
From: David Heidelberg @ 2026-06-27 18:41 UTC (permalink / raw)
  To: Samuel Page
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, oe-linux-nfc, netdev, linux-kernel, stable

On 26/06/2026 11:03, Samuel Page wrote:
> nci_rf_discover_ntf_packet() and nci_rf_intf_activated_ntf_packet() each
> parse a notification into an on-stack struct (nci_rf_discover_ntf /
> nci_rf_intf_activated_ntf) that is not initialised. The RF
> technology-specific parameters are only extracted when
> rf_tech_specific_params_len is non-zero, so a notification that reports a
> zero length leaves the rf_tech_specific_params union uninitialised - and
> both handlers then pass it to nci_add_new_protocol(), which reads it:
> 
>   - discover:  nci_add_new_target() -> nci_add_new_protocol();
>   - activated: nci_target_auto_activated() -> nci_add_new_protocol().
> 
> nci_add_new_protocol() uses nfca_poll->nfcid1_len as both a branch
> condition and a memcpy() length and copies nfcid1/sens_res/sel_res into
> ndev->targets, which is later exposed to user space via NFC_CMD_GET_TARGET.
> 
>    BUG: KMSAN: uninit-value in nci_add_new_protocol+0x624/0x6c0
>     nci_add_new_protocol+0x624/0x6c0
>     nci_ntf_packet+0x25b2/0x3c30
>     nci_rx_work+0x318/0x5d0
>     process_scheduled_works+0x84b/0x17a0
>     worker_thread+0xc10/0x11b0
>     kthread+0x376/0x500
>    Local variable ntf.i created at:
>     nci_ntf_packet+0xbc2/0x3c30
> 
> Zero-initialise both on-stack notifications so the union reads back as
> zero when no technology-specific parameters are present.
> 
> Fixes: 019c4fbaa790 ("NFC: Add NCI multiple targets support")
> Fixes: e8c0dacd9836 ("NFC: Update names and structs to NCI spec 1.0 d18")
> Link: https://lore.kernel.org/netdev/20260623172109.1105965-2-horms@kernel.org/
> Cc: stable@vger.kernel.org
> Assisted-by: Bynario AI

Hello Samuel,

the fix look good, may I ask you to follow the Assisted-by syntax as requested 
in [1]?

Thank you
David

[1] https://docs.kernel.org/process/coding-assistants.html

> Signed-off-by: Samuel Page <sam@bynar.io>
> ---
> v2: Drop the inaccurate activation_params / NFC_ATTR_TARGET_ATS scenario
>      from the commit message. No code change; the ntf = {} fix is unchanged.
> 
>   net/nfc/nci/ntf.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

[...]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-27 18:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26  9:03 [PATCH net v2] nfc: nci: fix uninit-value in the RF discover/activated NTF handlers Samuel Page
2026-06-27 18:41 ` David Heidelberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox