nfc: llcp: Fix use-after-free in llcp_sock_release()

PATCH In review Lee Jones 4 participants 8 messages Apr 29 – May 6

Subsystems (3)

Networking Network drivers NFC

lore

Share & tools

lore

Lee Jones

author

2026-04-29 13:41 UTC #0 lore raw

PATCH 1/2 nfc: llcp: Fix use-after-free in llcp_sock_release()

llcp_sock_release() unconditionally unlinks the socket from the local sockets list. However, if the socket is still in connecting state, it is on the connecting list.

Fix this by checking the socket state and unlinking from the correct list.

Fixes b4011239a08e ("NFC: llcp: Fix non blocking sockets connections")

Signed-off-by Lee Jones <[email protected]>

---
 net/nfc/llcp_sock.c | 2 ++
 1 file changed, 2 insertions(+)

net/nfc/llcp_sock.c +2 -0

index f1be1e84f6653..feab29fc62f44 100644
@@ -633,6 +633,8 @@ static int llcp_sock_release(struct socket *sock)
 
 	if (sock->type == SOCK_RAW)
 		nfc_llcp_sock_unlink(&local->raw_sockets, sk);
+	else if (sk->sk_state == LLCP_CONNECTING)
+		nfc_llcp_sock_unlink(&local->connecting_sockets, sk);
 	else
 		nfc_llcp_sock_unlink(&local->sockets, sk);

Signature · 1 line

2.54.0.545.g6539524ca2-goog

Lee Jones

author

2026-04-29 13:41 UTC #1 lore raw

PATCH 2/2 nfc: llcp: Fix use-after-free race in nfc_llcp_recv_cc()

A race condition exists in the NFC LLCP connection state machine where the connection acceptance packet (CC) can be processed concurrently with socket release. This can lead to a use-after-free of the socket object.

When nfc_llcp_recv_cc() moves the socket from the connecting_sockets list to the sockets list, it does so without holding the socket lock. If llcp_sock_release() is executing concurrently, it might have already unlinked the socket and dropped its references, which can result in nfc_llcp_recv_cc() linking a freed socket into the live list.

Fix this by holding lock_sock() during the state transition and list movement in nfc_llcp_recv_cc(). After acquiring the lock, check if the socket is still hashed to ensure it hasn't already been unlinked and marked for destruction by the release path. This aligns the locking pattern with recv_hdlc() and recv_disc().

Fixes a69f32af86e3 ("NFC: Socket linked list")

Signed-off-by Lee Jones <[email protected]>

---
 net/nfc/llcp_core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

net/nfc/llcp_core.c +11 -0

index db5bc6a878ddb..dc65c719f35f2 100644
@@ -1218,6 +1218,15 @@ static void nfc_llcp_recv_cc(struct nfc_llcp_local *local,
 
 	sk = &llcp_sock->sk;
 
+	lock_sock(sk);
+
+	/* Check if socket was destroyed whilst waiting for the lock */
+	if (!sk_hashed(sk)) {
+		release_sock(sk);
+		nfc_llcp_sock_put(llcp_sock);
+		return;
+	}
+
 	/* Unlink from connecting and link to the client array */
 	nfc_llcp_sock_unlink(&local->connecting_sockets, sk);
 	nfc_llcp_sock_link(&local->sockets, sk);

@@ -1229,6 +1238,8 @@ static void nfc_llcp_recv_cc(struct nfc_llcp_local *local,
 	sk->sk_state = LLCP_CONNECTED;
 	sk->sk_state_change(sk);
 
+	release_sock(sk);
+
 	nfc_llcp_sock_put(llcp_sock);
 }

Signature · 1 line

2.54.0.545.g6539524ca2-goog

Simon Horman

reviewer Reviewed-by on v1 2/2

2026-05-01 13:28 UTC #2 lore raw

Re: [PATCH 2/2] nfc: llcp: Fix use-after-free race in nfc_llcp_recv_cc()

Quoted 10 lines · Lee Jones

A race condition exists in the NFC LLCP connection state machine where the connection acceptance packet (CC) can be processed concurrently with socket release. This can lead to a use-after-free of the socket object.
When nfc_llcp_recv_cc() moves the socket from the connecting_sockets list to the sockets list, it does so without holding the socket lock. If llcp_sock_release() is executing concurrently, it might have already unlinked the socket and dropped its references, which can result in nfc_llcp_recv_cc() linking a freed socket into the live list.

Quoted 8 lines

Fix this by holding lock_sock() during the state transition and list movement in nfc_llcp_recv_cc(). After acquiring the lock, check if the socket is still hashed to ensure it hasn't already been unlinked and marked for destruction by the release path. This aligns the locking pattern with recv_hdlc() and recv_disc().
Fixes a69f32af86e3 ("NFC: Socket linked list")
Signed-off-by Lee Jones <[email protected]>

Reviewed-by Simon Horman <[email protected]>

FTR, there is an AI generated review available for this patch on sashiko.dev. I have looked over it and I believe it only covers pre-existing issues and should not block progress of this patch.

Simon Horman

reviewer

2026-05-01 13:01 UTC #3 lore raw

In reply to Lee Jones

From: 'Simon Horman' <[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://sashiko.dev
---
nfc: llcp: Fix use-after-free in llcp_sock_release()

This commit addresses a use-after-free issue in llcp_sock_release(). It attempts to fix the problem by checking if the socket is in the connecting state, and if so, unlinks it from the connecting_sockets list instead of the default sockets list.

Quoted 4 lines

net/nfc/llcp_sock.c +0 -0
index f1be1e84f665..feab29fc62f4 100644

[ ... ]

net/nfc/llcp_sock.c static int llcp_sock_release(struct socket *sock) · line 637

Quoted 6 lines

net/nfc/llcp_sock.c +2 -0

@@ -633,6 +633,8 @@ static int llcp_sock_release(struct socket *sock)
 
 	if (sock->type == SOCK_RAW)
 		nfc_llcp_sock_unlink(&local->raw_sockets, sk);
+	else if (sk->sk_state == LLCP_CONNECTING)
+		nfc_llcp_sock_unlink(&local->connecting_sockets, sk);

Does this cover the case where a remote peer rejects a non-blocking connection with a DM frame?

If a non-blocking connection is initiated, the socket is placed on local->connecting_sockets with the state LLCP_CONNECTING. If the remote peer then rejects the connection via a Disconnect Mode (DM) frame, nfc_llcp_recv_dm() sets sk->sk_state = LLCP_CLOSED but leaves the socket in the connecting_sockets list.

When llcp_sock_release() later checks the state, it will be LLCP_CLOSED, causing the new LLCP_CONNECTING check to fail. The code will then fall through to the else branch:

net/nfc/llcp_sock.c static int llcp_sock_release(struct socket *sock) · line 639

Quoted 2 lines

net/nfc/llcp_sock.c +0 -0

 	else
 		nfc_llcp_sock_unlink(&local->sockets, sk);

Could this corrupt the list? The nfc_llcp_sock_unlink() call for local->sockets acquires local->sockets.lock and executes sk_del_node_init(). Since the socket is physically still on the local->connecting_sockets hlist, this would mutate the connecting_sockets list pointers concurrently without holding the correct local->connecting_sockets.lock.

Simon says: while I think the problem above warrants looking into in
the context of this patch, I think the problem below pre-dates this patch and does not need to block progress of it.

Is there also a race condition here with nfc_llcp_recv_cc()?

In nfc_llcp_recv_cc(), a connecting socket is unlinked from connecting_sockets and linked to sockets without acquiring lock_sock(sk).

If llcp_sock_release() executes concurrently (since it holds lock_sock(sk) while recv_cc does not), could it fully unlink the socket and drop its references while nfc_llcp_recv_cc() resumes, blindly linking the now released socket into local->sockets?

This would leave a dangling pointer in the local->sockets list. I noticed this specific race appears to be addressed later in the series by commit e4b0d26ffad8cf0db2a59991b8b7098890b74187 ("nfc: llcp: Fix use-after-free race in nfc_llcp_recv_cc()").

Jakub Kicinski

maintainer

2026-05-01 23:28 UTC #4 lore raw

In reply to Lee Jones

Quoted 8 lines · On Wed, 29 Apr 2026 13:40:41 +0000 Lee Jones wrote:

llcp_sock_release() unconditionally unlinks the socket from the local sockets list. However, if the socket is still in connecting state, it is on the connecting list.
Fix this by checking the socket state and unlinking from the correct list.
Fixes b4011239a08e ("NFC: llcp: Fix non blocking sockets connections")
Signed-off-by Lee Jones <[email protected]>

Adding David H and dropping from netdev's patchwork..

Lee Jones

author

2026-05-06 08:11 UTC #5 lore raw

Quoted 11 lines · Jakub Kicinski

Quoted 8 lines · On Wed, 29 Apr 2026 13:40:41 +0000 Lee Jones wrote:
llcp_sock_release() unconditionally unlinks the socket from the local sockets list. However, if the socket is still in connecting state, it is on the connecting list.
Fix this by checking the socket state and unlinking from the correct list.
Fixes b4011239a08e ("NFC: llcp: Fix non blocking sockets connections")
Signed-off-by Lee Jones <[email protected]>
Adding David H and dropping from netdev's patchwork..

Is anyone looking at these please?

These are pretty important.

Signature · 1 line

Lee Jones

David Heidelberg

maintainer

2026-05-06 11:09 UTC #6 lore raw

Hello Lee.

Yeah, I think today these should hit the linux-next integration tree, and I need to setup the Thank you email to work in b4 review :)

David

-------- Original Message --------
From: Lee Jones <[email protected]>
Sent: 6 May 2026 08:11:45 UTC
To: Jakub Kicinski <[email protected]>
Cc: David Heidelberg <[email protected]>, "David S. Miller" <[email protected]>, Eric Dumazet <[email protected]>, Paolo Abeni <[email protected]>, Simon Horman <[email protected]>, Kuniyuki Iwashima <[email protected]>, Kees Cook <[email protected]>, Junxi Qian <[email protected]>, Ingo Molnar <[email protected]>, Samuel Ortiz <[email protected]>, [email protected], [email protected]
Subject: Re: [PATCH 1/2] nfc: llcp: Fix use-after-free in llcp_sock_release()

Quoted 11 lines · Jakub Kicinski

Quoted 8 lines · On Wed, 29 Apr 2026 13:40:41 +0000 Lee Jones wrote:
llcp_sock_release() unconditionally unlinks the socket from the local sockets list. However, if the socket is still in connecting state, it is on the connecting list.
Fix this by checking the socket state and unlinking from the correct list.
Fixes b4011239a08e ("NFC: llcp: Fix non blocking sockets connections")
Signed-off-by Lee Jones <[email protected]>
Adding David H and dropping from netdev's patchwork..

Is anyone looking at these please?

These are pretty important.

Lee Jones

author

2026-05-06 13:52 UTC #7 lore raw

Quoted 3 lines · David Heidelberg

Hello Lee.
Yeah, I think today these should hit the linux-next integration tree, and I need to setup the Thank you email to work in b4 review :)

Thanks David. And thanks for picking up the new role.

BTW, you may want to configure your mailer too. :)

Quoted 25 lines

-------- Original Message --------
From: Lee Jones <[email protected]>
Sent: 6 May 2026 08:11:45 UTC
To: Jakub Kicinski <[email protected]>
Cc: David Heidelberg <[email protected]>, "David S. Miller" <[email protected]>, Eric Dumazet <[email protected]>, Paolo Abeni <[email protected]>, Simon Horman <[email protected]>, Kuniyuki Iwashima <[email protected]>, Kees Cook <[email protected]>, Junxi Qian <[email protected]>, Ingo Molnar <[email protected]>, Samuel Ortiz <[email protected]>, [email protected], [email protected]
Subject: Re: [PATCH 1/2] nfc: llcp: Fix use-after-free in llcp_sock_release()
Quoted 11 lines · Jakub Kicinski
Quoted 8 lines · On Wed, 29 Apr 2026 13:40:41 +0000 Lee Jones wrote:
llcp_sock_release() unconditionally unlinks the socket from the local sockets list. However, if the socket is still in connecting state, it is on the connecting list.
Fix this by checking the socket state and unlinking from the correct list.
Fixes b4011239a08e ("NFC: llcp: Fix non blocking sockets connections")
Signed-off-by Lee Jones <[email protected]>
Adding David H and dropping from netdev's patchwork..
Is anyone looking at these please?
These are pretty important.

Signature · 1 line

Lee Jones