[PATCH net 0/5] nfc: fix leaks and races surfaced by NIPA

[PATCH net 0/5] nfc: fix leaks and races surfaced by NIPA

[Jakub Kicinski]

I recently added the nci test to NIPA. Somewhat surprisingly it runs
without much settup but hits kmemleaks fairly often. Fix a handful of
issues to make the test pass in a stable way.

Jakub Kicinski (5):
  nfc: nci: free skb on nci_transceive early error paths
  nfc: digital: free skb on digital_in_send error paths
  nfc: nci: complete pending data exchange on device close
  nfc: nci: clear NCI_DATA_EXCHANGE before calling completion callback
  nfc: rawsock: cancel tx_work before socket teardown

 net/nfc/digital_core.c |  8 ++++++--
 net/nfc/nci/core.c     | 18 ++++++++++++++++--
 net/nfc/nci/data.c     | 12 ++++++++----
 net/nfc/rawsock.c      | 11 +++++++++++
 4 files changed, 41 insertions(+), 8 deletions(-)

-- 
2.53.0

[PATCH net 1/5] nfc: nci: free skb on nci_transceive early error paths

[Jakub Kicinski]

nci_transceive() takes ownership of the skb passed by the caller,
but the -EPROTO, -EINVAL, and -EBUSY error paths return without
freeing it.

Due to issues clearing NCI_DATA_EXCHANGE fixed by subsequent changes
the nci/nci_dev selftest hits the error path occasionally in NIPA,
and kmemleak detects leaks:

unreferenced object 0xff11000015ce6a40 (size 640):
  comm "nci_dev", pid 3954, jiffies 4295441246
  hex dump (first 32 bytes):
    6b 6b 6b 6b 00 a4 00 0c 02 e1 03 6b 6b 6b 6b 6b  kkkk.......kkkkk
    6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
  backtrace (crc 7c40cc2a):
    kmem_cache_alloc_node_noprof+0x492/0x630
    __alloc_skb+0x11e/0x5f0
    alloc_skb_with_frags+0xc6/0x8f0
    sock_alloc_send_pskb+0x326/0x3f0
    nfc_alloc_send_skb+0x94/0x1d0
    rawsock_sendmsg+0x162/0x4c0
    do_syscall_64+0x117/0xfc0

Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/nfc/nci/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 6e9b76e2cc56..40fc397858ce 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -1035,18 +1035,23 @@ static int nci_transceive(struct nfc_dev *nfc_dev, struct nfc_target *target,
 	struct nci_conn_info *conn_info;
 
 	conn_info = ndev->rf_conn_info;
-	if (!conn_info)
+	if (!conn_info) {
+		kfree_skb(skb);
 		return -EPROTO;
+	}
 
 	pr_debug("target_idx %d, len %d\n", target->idx, skb->len);
 
 	if (!ndev->target_active_prot) {
 		pr_err("unable to exchange data, no active target\n");
+		kfree_skb(skb);
 		return -EINVAL;
 	}
 
-	if (test_and_set_bit(NCI_DATA_EXCHANGE, &ndev->flags))
+	if (test_and_set_bit(NCI_DATA_EXCHANGE, &ndev->flags)) {
+		kfree_skb(skb);
 		return -EBUSY;
+	}
 
 	/* store cb and context to be used on receiving data */
 	conn_info->data_exchange_cb = cb;
-- 
2.53.0

[PATCH net 2/5] nfc: digital: free skb on digital_in_send error paths

[Jakub Kicinski]

digital_in_send() takes ownership of the skb passed by the caller
(nfc_data_exchange), make sure it's freed on all error paths.

Found looking around the real driver for similar bugs to the one
just fixed in nci.

Fixes: 7d0911c02fa2 ("NFC: digital: Add NFC-DEP Initiator-side send/receive support")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/nfc/digital_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/nfc/digital_core.c b/net/nfc/digital_core.c
index 3670bb33732e..7cb1e6aaae90 100644
--- a/net/nfc/digital_core.c
+++ b/net/nfc/digital_core.c
@@ -707,8 +707,10 @@ static int digital_in_send(struct nfc_dev *nfc_dev, struct nfc_target *target,
 	int rc;
 
 	data_exch = kzalloc_obj(*data_exch);
-	if (!data_exch)
+	if (!data_exch) {
+		kfree_skb(skb);
 		return -ENOMEM;
+	}
 
 	data_exch->cb = cb;
 	data_exch->cb_context = cb_context;
@@ -731,8 +733,10 @@ static int digital_in_send(struct nfc_dev *nfc_dev, struct nfc_target *target,
 				 data_exch);
 
 exit:
-	if (rc)
+	if (rc) {
+		kfree_skb(skb);
 		kfree(data_exch);
+	}
 
 	return rc;
 }
-- 
2.53.0

[PATCH net 3/5] nfc: nci: complete pending data exchange on device close

[Jakub Kicinski]

In nci_close_device(), complete any pending data exchange before
closing. The data exchange callback (e.g.
rawsock_data_exchange_complete) holds a socket reference.

NIPA occasionally hits this leak:

unreferenced object 0xff1100000f435000 (size 2048):
  comm "nci_dev", pid 3954, jiffies 4295441245
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    27 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  '..@............
  backtrace (crc ec2b3c5):
    __kmalloc_noprof+0x4db/0x730
    sk_prot_alloc.isra.0+0xe4/0x1d0
    sk_alloc+0x36/0x760
    rawsock_create+0xd1/0x540
    nfc_sock_create+0x11f/0x280
    __sock_create+0x22d/0x630
    __sys_socket+0x115/0x1d0
    __x64_sys_socket+0x72/0xd0
    do_syscall_64+0x117/0xfc0
    entry_SYSCALL_64_after_hwframe+0x4b/0x53

Fixes: 38f04c6b1b68 ("NFC: protect nci_data_exchange transactions")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/nfc/nci/core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 40fc397858ce..f8c0bab2ec07 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -567,6 +567,10 @@ static int nci_close_device(struct nci_dev *ndev)
 		flush_workqueue(ndev->cmd_wq);
 		timer_delete_sync(&ndev->cmd_timer);
 		timer_delete_sync(&ndev->data_timer);
+		if (test_bit(NCI_DATA_EXCHANGE, &ndev->flags))
+			nci_data_exchange_complete(ndev, NULL,
+						   ndev->cur_conn_id,
+						   -ENODEV);
 		mutex_unlock(&ndev->req_lock);
 		return 0;
 	}
@@ -598,6 +602,11 @@ static int nci_close_device(struct nci_dev *ndev)
 	flush_workqueue(ndev->cmd_wq);
 
 	timer_delete_sync(&ndev->cmd_timer);
+	timer_delete_sync(&ndev->data_timer);
+
+	if (test_bit(NCI_DATA_EXCHANGE, &ndev->flags))
+		nci_data_exchange_complete(ndev, NULL, ndev->cur_conn_id,
+					   -ENODEV);
 
 	/* Clear flags except NCI_UNREG */
 	ndev->flags &= BIT(NCI_UNREG);
-- 
2.53.0

[PATCH net 4/5] nfc: nci: clear NCI_DATA_EXCHANGE before calling completion callback

[Jakub Kicinski]

Move clear_bit(NCI_DATA_EXCHANGE) before invoking the data exchange
callback in nci_data_exchange_complete().

The callback (e.g. rawsock_data_exchange_complete) may immediately
schedule another data exchange via schedule_work(tx_work).  On a
multi-CPU system, tx_work can run and reach nci_transceive() before
the current nci_data_exchange_complete() clears the flag, causing
test_and_set_bit(NCI_DATA_EXCHANGE) to return -EBUSY and the new
transfer to fail.

This causes intermittent flakes in nci/nci_dev in NIPA:

  # #  RUN           NCI.NCI1_0.t4t_tag_read ...
  # # t4t_tag_read: Test terminated by timeout
  # #          FAIL  NCI.NCI1_0.t4t_tag_read
  # not ok 3 NCI.NCI1_0.t4t_tag_read

Fixes: 38f04c6b1b68 ("NFC: protect nci_data_exchange transactions")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/nfc/nci/data.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/nfc/nci/data.c b/net/nfc/nci/data.c
index 78f4131af3cf..efb001b41aa8 100644
--- a/net/nfc/nci/data.c
+++ b/net/nfc/nci/data.c
@@ -33,7 +33,8 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
 	conn_info = nci_get_conn_info_by_conn_id(ndev, conn_id);
 	if (!conn_info) {
 		kfree_skb(skb);
-		goto exit;
+		clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
+		return;
 	}
 
 	cb = conn_info->data_exchange_cb;
@@ -45,6 +46,12 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
 	timer_delete_sync(&ndev->data_timer);
 	clear_bit(NCI_DATA_EXCHANGE_TO, &ndev->flags);
 
+	/* Mark the exchange as done before calling the callback.
+	 * The callback (e.g. rawsock_data_exchange_complete) may
+	 * want to immediately queue another data exchange.
+         */
+	clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
+
 	if (cb) {
 		/* forward skb to nfc core */
 		cb(cb_context, skb, err);
@@ -54,9 +61,6 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
 		/* no waiting callback, free skb */
 		kfree_skb(skb);
 	}
-
-exit:
-	clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
 }
 
 /* ----------------- NCI TX Data ----------------- */
-- 
2.53.0

[PATCH net 5/5] nfc: rawsock: cancel tx_work before socket teardown

[Jakub Kicinski]

In rawsock_release(), cancel any pending tx_work and purge the write
queue before orphaning the socket.  rawsock_tx_work runs on the system
workqueue and calls nfc_data_exchange which dereferences the NCI
device.  Without synchronization, tx_work can race with socket and
device teardown when a process is killed (e.g. by SIGKILL), leading
to use-after-free or leaked references.

Set SEND_SHUTDOWN first so that if tx_work is already running it will
see the flag and skip transmitting, then use cancel_work_sync to wait
for any in-progress execution to finish, and finally purge any
remaining queued skbs.

Fixes: 23b7869c0fd0 ("NFC: add the NFC socket raw protocol")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/nfc/rawsock.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index b049022399ae..f7d7a599fade 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -67,6 +67,17 @@ static int rawsock_release(struct socket *sock)
 	if (sock->type == SOCK_RAW)
 		nfc_sock_unlink(&raw_sk_list, sk);
 
+	if (sk->sk_state == TCP_ESTABLISHED) {
+		/* Prevent rawsock_tx_work from starting new transmits and
+		 * wait for any in-progress work to finish.  This must happen
+		 * before the socket is orphaned to avoid a race where
+		 * rawsock_tx_work runs after the NCI device has been freed.
+		 */
+		sk->sk_shutdown |= SEND_SHUTDOWN;
+		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
+		rawsock_write_queue_purge(sk);
+	}
+
 	sock_orphan(sk);
 	sock_put(sk);
 
-- 
2.53.0

Re: [PATCH net 1/5] nfc: nci: free skb on nci_transceive early error paths

[Joe Damato]

On Tue, Mar 03, 2026 at 08:23:41AM -0800, Jakub Kicinski wrote:
> nci_transceive() takes ownership of the skb passed by the caller,
> but the -EPROTO, -EINVAL, and -EBUSY error paths return without
> freeing it.
> 
> Due to issues clearing NCI_DATA_EXCHANGE fixed by subsequent changes
> the nci/nci_dev selftest hits the error path occasionally in NIPA,
> and kmemleak detects leaks:
> 
> unreferenced object 0xff11000015ce6a40 (size 640):
>   comm "nci_dev", pid 3954, jiffies 4295441246
>   hex dump (first 32 bytes):
>     6b 6b 6b 6b 00 a4 00 0c 02 e1 03 6b 6b 6b 6b 6b  kkkk.......kkkkk
>     6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b  kkkkkkkkkkkkkkkk
>   backtrace (crc 7c40cc2a):
>     kmem_cache_alloc_node_noprof+0x492/0x630
>     __alloc_skb+0x11e/0x5f0
>     alloc_skb_with_frags+0xc6/0x8f0
>     sock_alloc_send_pskb+0x326/0x3f0
>     nfc_alloc_send_skb+0x94/0x1d0
>     rawsock_sendmsg+0x162/0x4c0
>     do_syscall_64+0x117/0xfc0
> 
> Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/nci/core.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net 2/5] nfc: digital: free skb on digital_in_send error paths

[Joe Damato]

On Tue, Mar 03, 2026 at 08:23:42AM -0800, Jakub Kicinski wrote:
> digital_in_send() takes ownership of the skb passed by the caller
> (nfc_data_exchange), make sure it's freed on all error paths.
> 
> Found looking around the real driver for similar bugs to the one
> just fixed in nci.
> 
> Fixes: 7d0911c02fa2 ("NFC: digital: Add NFC-DEP Initiator-side send/receive support")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/digital_core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net 3/5] nfc: nci: complete pending data exchange on device close

[Joe Damato]

On Tue, Mar 03, 2026 at 08:23:43AM -0800, Jakub Kicinski wrote:
> In nci_close_device(), complete any pending data exchange before
> closing. The data exchange callback (e.g.
> rawsock_data_exchange_complete) holds a socket reference.
> 
> NIPA occasionally hits this leak:
> 
> unreferenced object 0xff1100000f435000 (size 2048):
>   comm "nci_dev", pid 3954, jiffies 4295441245
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     27 00 01 40 00 00 00 00 00 00 00 00 00 00 00 00  '..@............
>   backtrace (crc ec2b3c5):
>     __kmalloc_noprof+0x4db/0x730
>     sk_prot_alloc.isra.0+0xe4/0x1d0
>     sk_alloc+0x36/0x760
>     rawsock_create+0xd1/0x540
>     nfc_sock_create+0x11f/0x280
>     __sock_create+0x22d/0x630
>     __sys_socket+0x115/0x1d0
>     __x64_sys_socket+0x72/0xd0
>     do_syscall_64+0x117/0xfc0
>     entry_SYSCALL_64_after_hwframe+0x4b/0x53
> 
> Fixes: 38f04c6b1b68 ("NFC: protect nci_data_exchange transactions")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/nci/core.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net 4/5] nfc: nci: clear NCI_DATA_EXCHANGE before calling completion callback

[Joe Damato]

On Tue, Mar 03, 2026 at 08:23:44AM -0800, Jakub Kicinski wrote:
> Move clear_bit(NCI_DATA_EXCHANGE) before invoking the data exchange
> callback in nci_data_exchange_complete().
> 
> The callback (e.g. rawsock_data_exchange_complete) may immediately
> schedule another data exchange via schedule_work(tx_work).  On a
> multi-CPU system, tx_work can run and reach nci_transceive() before
> the current nci_data_exchange_complete() clears the flag, causing
> test_and_set_bit(NCI_DATA_EXCHANGE) to return -EBUSY and the new
> transfer to fail.
> 
> This causes intermittent flakes in nci/nci_dev in NIPA:
> 
>   # #  RUN           NCI.NCI1_0.t4t_tag_read ...
>   # # t4t_tag_read: Test terminated by timeout
>   # #          FAIL  NCI.NCI1_0.t4t_tag_read
>   # not ok 3 NCI.NCI1_0.t4t_tag_read
> 
> Fixes: 38f04c6b1b68 ("NFC: protect nci_data_exchange transactions")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/nci/data.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net 5/5] nfc: rawsock: cancel tx_work before socket teardown

[Joe Damato]

On Tue, Mar 03, 2026 at 08:23:45AM -0800, Jakub Kicinski wrote:
> In rawsock_release(), cancel any pending tx_work and purge the write
> queue before orphaning the socket.  rawsock_tx_work runs on the system
> workqueue and calls nfc_data_exchange which dereferences the NCI
> device.  Without synchronization, tx_work can race with socket and
> device teardown when a process is killed (e.g. by SIGKILL), leading
> to use-after-free or leaked references.
> 
> Set SEND_SHUTDOWN first so that if tx_work is already running it will
> see the flag and skip transmitting, then use cancel_work_sync to wait
> for any in-progress execution to finish, and finally purge any
> remaining queued skbs.
> 
> Fixes: 23b7869c0fd0 ("NFC: add the NFC socket raw protocol")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/rawsock.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>

Reviewed-by: Joe Damato <joe@dama.to>

Re: [PATCH net 0/5] nfc: fix leaks and races surfaced by NIPA

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  3 Mar 2026 08:23:40 -0800 you wrote:
> I recently added the nci test to NIPA. Somewhat surprisingly it runs
> without much settup but hits kmemleaks fairly often. Fix a handful of
> issues to make the test pass in a stable way.
> 
> Jakub Kicinski (5):
>   nfc: nci: free skb on nci_transceive early error paths
>   nfc: digital: free skb on digital_in_send error paths
>   nfc: nci: complete pending data exchange on device close
>   nfc: nci: clear NCI_DATA_EXCHANGE before calling completion callback
>   nfc: rawsock: cancel tx_work before socket teardown
> 
> [...]

Here is the summary with links:
  - [net,1/5] nfc: nci: free skb on nci_transceive early error paths
    https://git.kernel.org/netdev/net/c/7bd4b0c4779f
  - [net,2/5] nfc: digital: free skb on digital_in_send error paths
    https://git.kernel.org/netdev/net/c/d42449d2c17c
  - [net,3/5] nfc: nci: complete pending data exchange on device close
    https://git.kernel.org/netdev/net/c/66083581945b
  - [net,4/5] nfc: nci: clear NCI_DATA_EXCHANGE before calling completion callback
    https://git.kernel.org/netdev/net/c/0efdc02f4f6d
  - [net,5/5] nfc: rawsock: cancel tx_work before socket teardown
    https://git.kernel.org/netdev/net/c/d793458c45df

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net 5/5] nfc: rawsock: cancel tx_work before socket teardown

[Guenter Roeck]

Hi,

On Tue, Mar 03, 2026 at 08:23:45AM -0800, Jakub Kicinski wrote:
> In rawsock_release(), cancel any pending tx_work and purge the write
> queue before orphaning the socket.  rawsock_tx_work runs on the system
> workqueue and calls nfc_data_exchange which dereferences the NCI
> device.  Without synchronization, tx_work can race with socket and
> device teardown when a process is killed (e.g. by SIGKILL), leading
> to use-after-free or leaked references.
> 
> Set SEND_SHUTDOWN first so that if tx_work is already running it will
> see the flag and skip transmitting, then use cancel_work_sync to wait
> for any in-progress execution to finish, and finally purge any
> remaining queued skbs.
> 
> Fixes: 23b7869c0fd0 ("NFC: add the NFC socket raw protocol")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/nfc/rawsock.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
> index b049022399ae..f7d7a599fade 100644
> --- a/net/nfc/rawsock.c
> +++ b/net/nfc/rawsock.c
> @@ -67,6 +67,17 @@ static int rawsock_release(struct socket *sock)
>  	if (sock->type == SOCK_RAW)
>  		nfc_sock_unlink(&raw_sk_list, sk);
>  
> +	if (sk->sk_state == TCP_ESTABLISHED) {
> +		/* Prevent rawsock_tx_work from starting new transmits and
> +		 * wait for any in-progress work to finish.  This must happen
> +		 * before the socket is orphaned to avoid a race where
> +		 * rawsock_tx_work runs after the NCI device has been freed.
> +		 */
> +		sk->sk_shutdown |= SEND_SHUTDOWN;
> +		cancel_work_sync(&nfc_rawsock(sk)->tx_work);
> +		rawsock_write_queue_purge(sk);
> +	}
> +

Google's AI agent had the following fedback on this patch:

  Does cancel_work_sync() properly wait for the asynchronous nfc_data_exchange()
  to complete?

  Since rawsock_tx_work() only initiates the exchange and then returns, the work
  item is no longer running or pending while the exchange is in flight. This
  means cancel_work_sync() might return immediately.

  > +		rawsock_write_queue_purge(sk);

  If the hardware completes the data exchange after cancel_work_sync() has
  returned, but before rawsock_write_queue_purge() empties the write queue,
  rawsock_data_exchange_complete() will see a non-empty queue and call
  schedule_work().

  Because cancel_work_sync() has already finished, this newly scheduled tx_work
  could execute after rawsock_release() has freed the socket. Can this lead
  to a use-after-free?

On top of that, I don't see how the this patch change would prevent the
problem it tries to fix if tx_work is running and has proceeded beyond
the SEND_SHUTDOWN check.

My apologies if this is all just noise. If so, please let me know what is
wrong with the analysis to help improve the agent.

Thanks,
Guenter