Changes in v3: - Wrap commit messages to <= 75 columns - Use 12-char SHA in Fixes tags - Verify Fixes with git blame (both handlers trace back to d646960f7986) - Run scripts/checkpatch.pl --strict
Qianchang Zhao (2): nfc: llcp: avoid double release/put on LLCP_CLOSED in nfc_llcp_recv_disc() nfc: llcp: stop processing on LLCP_CLOSED in nfc_llcp_recv_hdlc()
net/nfc/llcp_core.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
nfc_llcp_sock_get() takes a reference on the LLCP socket via sock_hold().
In nfc_llcp_recv_disc(), when the socket is already in LLCP_CLOSED state, the code used to perform release_sock() and nfc_llcp_sock_put() in the CLOSED branch but then continued execution and later performed the same cleanup again on the common exit path. This results in refcount imbalance (double put) and unbalanced lock release.
Remove the redundant CLOSED-branch cleanup so that release_sock() and nfc_llcp_sock_put() are performed exactly once via the common exit path, while keeping the existing DM_DISC reply behavior.
Fixes: d646960f7986 ("NFC: Initial LLCP support") Cc: stable@vger.kernel.org Signed-off-by: Qianchang Zhao pioooooooooip@gmail.com --- net/nfc/llcp_core.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index beeb3b4d2..ed37604ed 100644 --- a/net/nfc/llcp_core.c +++ b/net/nfc/llcp_core.c @@ -1177,11 +1177,6 @@ static void nfc_llcp_recv_disc(struct nfc_llcp_local *local,
nfc_llcp_socket_purge(llcp_sock);
- if (sk->sk_state == LLCP_CLOSED) { - release_sock(sk); - nfc_llcp_sock_put(llcp_sock); - } - if (sk->sk_state == LLCP_CONNECTED) { nfc_put_device(local->dev); sk->sk_state = LLCP_CLOSED;
On 12/18/25 3:59 AM, Qianchang Zhao wrote:
nfc_llcp_sock_get() takes a reference on the LLCP socket via sock_hold().
In nfc_llcp_recv_disc(), when the socket is already in LLCP_CLOSED state, the code used to perform release_sock() and nfc_llcp_sock_put() in the CLOSED branch but then continued execution and later performed the same cleanup again on the common exit path. This results in refcount imbalance (double put) and unbalanced lock release.
Remove the redundant CLOSED-branch cleanup so that release_sock() and nfc_llcp_sock_put() are performed exactly once via the common exit path, while keeping the existing DM_DISC reply behavior.
Fixes: d646960f7986 ("NFC: Initial LLCP support") Cc: stable@vger.kernel.org Signed-off-by: Qianchang Zhao pioooooooooip@gmail.com
net/nfc/llcp_core.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index beeb3b4d2..ed37604ed 100644 --- a/net/nfc/llcp_core.c +++ b/net/nfc/llcp_core.c @@ -1177,11 +1177,6 @@ static void nfc_llcp_recv_disc(struct nfc_llcp_local *local, nfc_llcp_socket_purge(llcp_sock);
- if (sk->sk_state == LLCP_CLOSED) {
release_sock(sk);nfc_llcp_sock_put(llcp_sock);
To rephrase Krzysztof concernt, this does not looks like the correct fix: later on nfc_llcp_recv_disc() will try a send over a closed socket, which looks wrong. Instead you could just return after nfc_llcp_sock_put(), or do something alike:
if (sk->sk_state == LLCP_CLOSED) goto cleanup;
// ...
cleanup: release_sock(sk); nfc_llcp_sock_put(llcp_sock); }
/P
On 12/28/25 10:02 AM, Paolo Abeni wrote:
On 12/18/25 3:59 AM, Qianchang Zhao wrote:
nfc_llcp_sock_get() takes a reference on the LLCP socket via sock_hold().
In nfc_llcp_recv_disc(), when the socket is already in LLCP_CLOSED state, the code used to perform release_sock() and nfc_llcp_sock_put() in the CLOSED branch but then continued execution and later performed the same cleanup again on the common exit path. This results in refcount imbalance (double put) and unbalanced lock release.
Remove the redundant CLOSED-branch cleanup so that release_sock() and nfc_llcp_sock_put() are performed exactly once via the common exit path, while keeping the existing DM_DISC reply behavior.
Fixes: d646960f7986 ("NFC: Initial LLCP support") Cc: stable@vger.kernel.org Signed-off-by: Qianchang Zhao pioooooooooip@gmail.com
net/nfc/llcp_core.c | 5 ----- 1 file changed, 5 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index beeb3b4d2..ed37604ed 100644 --- a/net/nfc/llcp_core.c +++ b/net/nfc/llcp_core.c @@ -1177,11 +1177,6 @@ static void nfc_llcp_recv_disc(struct nfc_llcp_local *local, nfc_llcp_socket_purge(llcp_sock);
- if (sk->sk_state == LLCP_CLOSED) {
release_sock(sk);nfc_llcp_sock_put(llcp_sock);To rephrase Krzysztof concernt, this does not looks like the correct fix: later on nfc_llcp_recv_disc() will try a send over a closed socket, which looks wrong. Instead you could just return after nfc_llcp_sock_put(), or do something alike:
if (sk->sk_state == LLCP_CLOSED) goto cleanup;
// ...
cleanup: release_sock(sk); nfc_llcp_sock_put(llcp_sock); }
I'm sorry for the confusing feedback above.
I read the comments on patch 2/2 only after processing this one.
Indeed following the half-interrupted discussion on old revision, with bad patch splitting is quite difficult.
@Qianchang Zhao: my _guess_ is that on LLCP_CLOSED the code has to release the final sk reference... In any case discussion an a patch series revision is not concluded until the reviewer agrees on that.
@Krzysztof: ... but still it looks like in the current code there is a double release on the sk socket lock, which looks wrong, what am I missing here?
/P
nfc_llcp_sock_get() takes a reference on the LLCP socket via sock_hold().
In nfc_llcp_recv_hdlc(), the LLCP_CLOSED branch releases the socket lock and drops the reference, but the function continues to operate on llcp_sock/sk and later runs release_sock() and nfc_llcp_sock_put() again on the common exit path.
Return immediately after the CLOSED cleanup to avoid refcount/lock imbalance and to avoid using the socket after dropping the reference.
Fixes: d646960f7986 ("NFC: Initial LLCP support") Cc: stable@vger.kernel.org Signed-off-by: Qianchang Zhao pioooooooooip@gmail.com --- net/nfc/llcp_core.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index ed37604ed..f6c1d79f9 100644 --- a/net/nfc/llcp_core.c +++ b/net/nfc/llcp_core.c @@ -1089,6 +1089,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 */
On 18/12/2025 03:59, Qianchang Zhao wrote:
Changes in v3:
- Wrap commit messages to <= 75 columns
- Use 12-char SHA in Fixes tags
- Verify Fixes with git blame (both handlers trace back to d646960f7986)
- Run scripts/checkpatch.pl --strict
Don't send new version while discussion is still going. You already did this with v2 (twice) and now with v3.
Best regards, Krzysztof
linux-stable-mirror@lists.linaro.org
-
Krzysztof Kozlowski -
Paolo Abeni -
Qianchang Zhao