[PATCH] nfc: llcp: fix missing return after LLCP_CLOSED check in recv_hdlc and recv_disc

[PATCH] nfc: llcp: fix missing return after LLCP_CLOSED check in recv_hdlc and recv_disc

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

nfc_llcp_recv_hdlc() and nfc_llcp_recv_disc() both call
nfc_llcp_sock_get() (which increments the socket reference count) and
lock_sock() before processing incoming PDUs.  When the socket is found
to be in state LLCP_CLOSED both functions correctly call release_sock()
and nfc_llcp_sock_put() to undo those operations, but are missing a
return statement:

    lock_sock(sk);
    if (sk->sk_state == LLCP_CLOSED) {
        release_sock(sk);
        nfc_llcp_sock_put(llcp_sock);
        /* ← return missing */
    }
    /* Falls through with lock released and reference dropped */
    ...
    release_sock(sk);            /* double unlock */
    nfc_llcp_sock_put(llcp_sock); /* double put → refcount underflow */

The fall-through causes three independent bugs:

  1. Use-after-free: all llcp_sock field accesses after the LLCP_CLOSED
     block occur with the socket lock released and the reference dropped;
     another CPU may free the socket concurrently.

  2. Double release_sock: sk_lock.owned is already 0 — LOCKDEP reports
     "WARNING: suspicious unlock balance detected".

  3. Double nfc_llcp_sock_put: the refcount is decremented a second time
     at the end of the function, potentially driving it below zero
     (refcount_t underflow), corrupting the SLUB freelist and causing a
     subsequent use-after-free or double-free.

Both functions are reachable from any NFC P2P peer within physical
proximity (~4 cm) without hostile NFCC firmware:
  - nfc_llcp_recv_hdlc: triggered by sending an LLCP I, RR, or RNR PDU
    to a SAP pair whose connection has been torn down.
  - nfc_llcp_recv_disc: triggered by sending an LLCP DISC PDU to a SAP
    pair that is already in LLCP_CLOSED state.

Fix: add the missing return statement in both functions so that the
LLCP_CLOSED branch exits after cleanup.

Fixes: Introduced with nfc_llcp_recv_hdlc / nfc_llcp_recv_disc
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/llcp_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 366d75663..db5bc6a87 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1091,6 +1091,7 @@ static void nfc_llcp_recv_hdlc(struct nfc_llcp_local *local,
 	if (sk->sk_state == LLCP_CLOSED) {
 		release_sock(sk);
 		nfc_llcp_sock_put(llcp_sock);
+		return;
 	}
 
 	/* Pass the payload upstream */
@@ -1182,6 +1183,7 @@ static void nfc_llcp_recv_disc(struct nfc_llcp_local *local,
 	if (sk->sk_state == LLCP_CLOSED) {
 		release_sock(sk);
 		nfc_llcp_sock_put(llcp_sock);
+		return;
 	}
 
 	if (sk->sk_state == LLCP_CONNECTED) {
-- 
2.51.0

Re: [PATCH] nfc: llcp: fix missing return after LLCP_CLOSED check in recv_hdlc and recv_disc

[Simon Horman]

On Sun, Apr 05, 2026 at 06:41:58PM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nfc_llcp_recv_hdlc() and nfc_llcp_recv_disc() both call
> nfc_llcp_sock_get() (which increments the socket reference count) and
> lock_sock() before processing incoming PDUs.  When the socket is found
> to be in state LLCP_CLOSED both functions correctly call release_sock()
> and nfc_llcp_sock_put() to undo those operations, but are missing a
> return statement:
> 
>     lock_sock(sk);
>     if (sk->sk_state == LLCP_CLOSED) {
>         release_sock(sk);
>         nfc_llcp_sock_put(llcp_sock);
>         /* ← return missing */
>     }
>     /* Falls through with lock released and reference dropped */
>     ...
>     release_sock(sk);            /* double unlock */
>     nfc_llcp_sock_put(llcp_sock); /* double put → refcount underflow */
> 
> The fall-through causes three independent bugs:
> 
>   1. Use-after-free: all llcp_sock field accesses after the LLCP_CLOSED
>      block occur with the socket lock released and the reference dropped;
>      another CPU may free the socket concurrently.
> 
>   2. Double release_sock: sk_lock.owned is already 0 — LOCKDEP reports
>      "WARNING: suspicious unlock balance detected".
> 
>   3. Double nfc_llcp_sock_put: the refcount is decremented a second time
>      at the end of the function, potentially driving it below zero
>      (refcount_t underflow), corrupting the SLUB freelist and causing a
>      subsequent use-after-free or double-free.
> 
> Both functions are reachable from any NFC P2P peer within physical
> proximity (~4 cm) without hostile NFCC firmware:
>   - nfc_llcp_recv_hdlc: triggered by sending an LLCP I, RR, or RNR PDU
>     to a SAP pair whose connection has been torn down.
>   - nfc_llcp_recv_disc: triggered by sending an LLCP DISC PDU to a SAP
>     pair that is already in LLCP_CLOSED state.
> 
> Fix: add the missing return statement in both functions so that the
> LLCP_CLOSED branch exits after cleanup.
> 
> Fixes: Introduced with nfc_llcp_recv_hdlc / nfc_llcp_recv_disc
> Signed-off-by: Lekë Hapçiu <framemain@outlook.com>

Curiously this seems to duplicate this patch:

- [PATCH net] nfc: llcp: add missing return after LLCP_CLOSED checks
  https://lore.kernel.org/all/20260408081006.3723-1-qjx1298677004@gmail.com/

-- 
pw-bot: changes-requested