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

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

[Lee Jones]

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 <lee@kernel.org>
---
 net/nfc/llcp_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index f1be1e84f6653..feab29fc62f44 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -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);
 
-- 
2.54.0.545.g6539524ca2-goog

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

[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.

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 <lee@kernel.org>
---
 net/nfc/llcp_core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index db5bc6a878ddb..dc65c719f35f2 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -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);
 }
 
-- 
2.54.0.545.g6539524ca2-goog

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

[Simon Horman]

On Wed, Apr 29, 2026 at 01:40:42PM +0000, Lee Jones wrote:
> 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 <lee@kernel.org>

Reviewed-by: Simon Horman <horms@kernel.org>

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.

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

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

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.

> diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
> index f1be1e84f665..feab29fc62f4 100644
> --- a/net/nfc/llcp_sock.c
> +++ b/net/nfc/llcp_sock.c
[ ... ]
> @@ -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:

>  	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()").

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

[Jakub Kicinski]

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 <lee@kernel.org>

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