[PATCH 0/1] nfc: llcp: reject connect() on sockets not in LLCP_CLOSED state

[PATCH 0/1] nfc: llcp: reject connect() on sockets not in LLCP_CLOSED state

[Yuchao Zhang]

This patch addresses a socket state validation gap in NFC LLCP socket
connection logic where connect() can be invoked on sockets in the
LLCP_BOUND (or LLCP_LISTEN) state.

Historical background and context:
Earlier patches addressing CVE-2020-25670 / CVE-2020-25673 added an
explicit check for the LLCP_CONNECTING state in llcp_sock_connect() to
prevent duplicate in-flight connection requests on non-blocking sockets
from linking the socket multiple times into local->connecting_sockets.

However, that check implicitly assumed that any socket that was neither
LLCP_CONNECTED nor LLCP_CONNECTING was in LLCP_CLOSED. It omitted the
possibility that a socket had already been bound via llcp_sock_bind(),
which transitions the socket to LLCP_BOUND and adds it to local->sockets.

Vulnerability impact:
When connect() is called on an already bound socket:
1. llcp_sock_connect() overwrites llcp_sock->dev and llcp_sock->local
   without dropping the original references, causing refcount and memory
   leaks (service_name).
2. The socket is added to local->connecting_sockets while still remaining
   linked in local->sockets.
3. Upon receiving connection confirmation (CC), nfc_llcp_recv_cc() unlinks
   the socket from connecting_sockets and links it into local->sockets.
   Because the head of local->sockets still pointed to the socket itself,
   hlist_add_head() links the node to itself (node->next = node), forming
   an infinite loop. Subsequent traversals of local->sockets (e.g. during
   lookup or release) loop indefinitely under spinlock, leading to kernel
   soft-lockups and RCU stalls.
4. If connect() targets a different device index, the socket becomes linked
   across two different NFC devices, causing cross-device socket confusion.

Fix:
Enforce that sockets must be in LLCP_CLOSED before initiating a connection,
returning -EBADFD otherwise, aligning with the state check behavior in
llcp_sock_bind() and llcp_raw_sock_bind().

Tested and confirmed style-compliant with checkpatch.pl
(0 errors, 0 warnings).

Yuchao Zhang (1):
  nfc: llcp: reject connect() on sockets not in LLCP_CLOSED state

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

-- 
2.53.0

[PATCH 1/1] nfc: llcp: reject connect() on sockets not in LLCP_CLOSED state

[Yuchao Zhang]

When a socket is bound via llcp_sock_bind(), its state transitions to
LLCP_BOUND and it is linked into the local->sockets hlist via
nfc_llcp_sock_link().

Currently, llcp_sock_connect() only checks if the socket is already
connected (LLCP_CONNECTED) or in progress (LLCP_CONNECTING):

	if (sk->sk_state == LLCP_CONNECTED) {
		ret = -EISCONN;
		goto error;
	}
	if (sk->sk_state == LLCP_CONNECTING) {
		ret = -EINPROGRESS;
		goto error;
	}

Crucially, it does not check for the LLCP_BOUND (or LLCP_LISTEN) state.
Previous fixes addressing CVE-2020-25670 / CVE-2020-25673 added the check
for LLCP_CONNECTING to prevent duplicate in-flight connection attempts
from corrupting the connecting_sockets list. However, they assumed any
other non-connected socket was in LLCP_CLOSED, leaving the LLCP_BOUND
state window unhandled.

If connect() is invoked on a socket that has already been bound:
1. llcp_sock_connect() silently overwrites llcp_sock->dev and
   llcp_sock->local without releasing previous references or freeing the
   bound service_name, leading to refcount and memory leaks.
2. The socket remains linked in local->sockets, while llcp_sock_connect()
   links it again into local->connecting_sockets via nfc_llcp_sock_link().
3. When connection confirmation (CC) arrives, nfc_llcp_recv_cc() executes:

	nfc_llcp_sock_unlink(&local->connecting_sockets, sk);
	nfc_llcp_sock_link(&local->sockets, sk);

   Because sk was never unlinked from local->sockets, the head.first of
   local->sockets still points to &sk->sk_node. The second
   hlist_add_head() sets sk->sk_node.next = sk->sk_node, producing a
   self-referential loop. Subsequent traversals of local->sockets
   (e.g. nfc_llcp_sock_get(), nfc_llcp_socket_release()) enter an
   infinite loop with spinlocks held, resulting in kernel soft-lockups.
4. Furthermore, if connect() specifies a different NFC device index than
   bind(), the socket remains linked on device A's socket list while
   being assigned to device B, causing cross-device socket confusion.

Fix this by ensuring that llcp_sock_connect() only accepts sockets in the
LLCP_CLOSED state, returning -EBADFD otherwise, matching the state check
behavior in llcp_sock_bind().

Fixes: d646960f7986 ("NFC: Initial LLCP support")
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 net/nfc/llcp_sock.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index 5558d8a4d48b..66f12e9af2fd 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -689,6 +689,10 @@ static int llcp_sock_connect(struct socket *sock, struct sockaddr_unsized *_addr
 		ret = -EINPROGRESS;
 		goto error;
 	}
+	if (sk->sk_state != LLCP_CLOSED) {
+		ret = -EBADFD;
+		goto error;
+	}
 
 	dev = nfc_get_device(addr->dev_idx);
 	if (dev == NULL) {
-- 
2.53.0