[PATCH net] nfc: llcp: fix NULL pointer dereference race in nfc_llcp_send_ui_frame()

[PATCH net] nfc: llcp: fix NULL pointer dereference race in nfc_llcp_send_ui_frame()

[Junwoong Doh]

nfc_llcp_send_ui_frame() checks whether sock->local is NULL, but it is
called by llcp_sock_sendmsg() without the socket lock held, which opens
a window for a race condition. Between the sock->local check and the
sock->dev use in nfc_alloc_send_skb(), llcp_sock_bind() can run
concurrently and set both sock->local and sock->dev to NULL, which can
lead to a NULL pointer dereference in nfc_alloc_send_skb().

Take the socket lock in nfc_llcp_send_ui_frame() so that the sock->local
check and the sock->dev use are performed under it. The message is
copied from user space before the lock is taken, to avoid holding the
lock across a user space access that can block for an unbounded amount
of time.

Fixes: dded08927ca3 ("nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()")
Signed-off-by: Junwoong Doh <jdoh.kernel@gmail.com>
Link: https://lore.kernel.org/all/a89d0419-8bcf-40a2-b52d-3e5d911f11da@gmail.com/
---
 net/nfc/llcp_commands.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 291f26facbf3..cfd5f6aebf8d 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -730,6 +730,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 			   struct msghdr *msg, size_t len)
 {
 	struct sk_buff *pdu;
+	struct sock *sk = &sock->sk;
 	struct nfc_llcp_local *local;
 	size_t frag_len = 0, remaining_len;
 	u8 *msg_ptr, *msg_data;
@@ -738,10 +739,6 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 
 	pr_debug("Send UI frame len %zd\n", len);
 
-	local = sock->local;
-	if (local == NULL)
-		return -ENODEV;
-
 	msg_data = kmalloc(len, GFP_USER | __GFP_NOWARN);
 	if (msg_data == NULL)
 		return -ENOMEM;
@@ -751,6 +748,15 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 		return -EFAULT;
 	}
 
+	lock_sock(sk);
+
+	local = sock->local;
+	if (local == NULL) {
+		release_sock(sk);
+		kfree(msg_data);
+		return -ENODEV;
+	}
+
 	remaining_len = len;
 	msg_ptr = msg_data;
 
@@ -763,7 +769,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 		pr_debug("Fragment %zd bytes remaining %zd",
 			 frag_len, remaining_len);
 
-		pdu = nfc_alloc_send_skb(sock->dev, &sock->sk, 0,
+		pdu = nfc_alloc_send_skb(sock->dev, sk, 0,
 					 frag_len + LLCP_HEADER_SIZE, &err);
 		if (pdu == NULL) {
 			pr_err("Could not allocate PDU (error=%d)\n", err);
@@ -800,6 +806,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 ssap, u8 dsap,
 		msg_ptr += frag_len;
 	} while (remaining_len > 0);
 
+	release_sock(sk);
 	kfree(msg_data);
 
 	return len;
-- 
2.34.1

Re: [PATCH net] nfc: llcp: fix NULL pointer dereference race in nfc_llcp_send_ui_frame()

[David Heidelberg]

On 25/07/2026 13:55, Junwoong Doh wrote:
> nfc_llcp_send_ui_frame() checks whether sock->local is NULL, but it is
> called by llcp_sock_sendmsg() without the socket lock held, which opens
> a window for a race condition. Between the sock->local check and the
> sock->dev use in nfc_alloc_send_skb(), llcp_sock_bind() can run
> concurrently and set both sock->local and sock->dev to NULL, which can
> lead to a NULL pointer dereference in nfc_alloc_send_skb().
> 
> Take the socket lock in nfc_llcp_send_ui_frame() so that the sock->local
> check and the sock->dev use are performed under it. The message is
> copied from user space before the lock is taken, to avoid holding the
> lock across a user space access that can block for an unbounded amount
> of time.
> 
> Fixes: dded08927ca3 ("nfc: llcp: fix NULL error pointer dereference on sendmsg() after failed bind()")
> Signed-off-by: Junwoong Doh <jdoh.kernel@gmail.com>
> Link: https://lore.kernel.org/all/a89d0419-8bcf-40a2-b52d-3e5d911f11da@gmail.com/
> ---
>   net/nfc/llcp_commands.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
> 
Hello Junwoong,

it seems Sashiko noted an issue, could you verify it please (see below, feel 
free to ignore the pre-existing one)?

Thanks
David

--- Patch [1]: [PATCH net] nfc: llcp: fix NULL pointer dereference race in 
nfc_llcp_send_ui_frame() ---
commit 9c4c4bdf57043772ddc74843af1f533e7e9bc0ca
Author: Junwoong Doh <jdoh.kernel@gmail.com>

nfc: llcp: fix NULL pointer dereference race in nfc_llcp_send_ui_frame()

This patch fixes a race condition in nfc_llcp_send_ui_frame() where
concurrent bind() calls can lead to a NULL pointer dereference. It moves
the allocation and socket state checks inside the socket lock.

Link: https://lore.kernel.org/all/a89d0419-8bcf-40a2-b52d-3e5d911f11da@gmail.com/

 > diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
 > index 291f26facbf3a..cfd5f6aebf8df 100644
 > --- a/net/nfc/llcp_commands.c
 > +++ b/net/nfc/llcp_commands.c
[ ... ]
 > @@ -763,7 +769,7 @@ int nfc_llcp_send_ui_frame(struct nfc_llcp_sock *sock, u8 
ssap, u8 dsap,
 >  		pr_debug("Fragment %zd bytes remaining %zd",
 >  			 frag_len, remaining_len);
 >
 > -		pdu = nfc_alloc_send_skb(sock->dev, &sock->sk, 0,
 > +		pdu = nfc_alloc_send_skb(sock->dev, sk, 0,
 >  					 frag_len + LLCP_HEADER_SIZE, &err);

Since the lock_sock(sk) critical section now covers this entire loop, could
calling nfc_alloc_send_skb() with a hardcoded flags value of 0 cause an
uninterruptible sleep deadlock?

When the socket's write buffer is full, sock_alloc_send_skb() will invoke
sock_wait_for_wmem() to sleep until memory is available. For datagram
sockets, this function does not drop the socket lock while sleeping.

Concurrently, any other operations needing the socket lock (such as close()
calling llcp_sock_release) might hang in a D-state indefinitely until the
send buffer drains.

 >  		if (pdu == NULL) {
 >  			pr_err("Could not allocate PDU (error=%d)\n", err);
 >  			len -= remaining_len;

This is a pre-existing issue, but does the exact same race condition remain
unpatched in nfc_llcp_send_i_frame()?

In nfc_llcp_send_i_frame(), llcp_allocate_pdu() is called without holding
the socket lock. If a concurrent disconnect frame transitions the socket to
LLCP_CLOSED, a concurrent failed bind() can set llcp_sock->dev to NULL:

net/nfc/llcp_commands.c:nfc_llcp_send_i_frame() {
     ...
     pdu = llcp_allocate_pdu(sock, LLCP_PDU_I,
                             frag_len + LLCP_SEQUENCE_SIZE);
     ...
}

Which then calls nfc_alloc_send_skb() and dereferences the nullified
sock->dev when computing the headroom:

net/nfc/core.c:nfc_alloc_send_skb() {
     ...
     total_size = size +
             dev->tx_headroom + dev->tx_tailroom + NFC_HEADER_SIZE;
     ...
}

Would it be necessary to apply a similar locking fix to
nfc_llcp_send_i_frame() to prevent this NULL pointer dereference?