I ask why do you think your code is correct.
Because the current code can execute release_sock()/nfc_llcp_sock_put() twice for the single ref acquired by nfc_llcp_sock_get(), and it can keep operating on llcp_sock/sk after the first put/unlock. The fix ensures exactly one cleanup (recv_disc) or exits immediately after the CLOSED cleanup (recv_hdlc), while keeping the existing DM_DISC send behavior.
The refcnt has imbalance only if you assume initial refcnt was 0.
No. Let baseline refcnt be N before nfc_llcp_sock_get(); sock_hold() makes N+1. CLOSED path put -> N, common exit put -> N-1. So it drops one extra ref regardless of N (whether it immediately frees depends on N). git blame shows both cleanup sites trace back to d646960f7986.
I will not send a new revision until this discussion is resolved.
Best regards, Qianchang
On Thu, Dec 18, 2025 at 7:22 PM Krzysztof Kozlowski krzk@kernel.org wrote:
On 17/12/2025 14:05, くさあさ wrote:
Hi Krzysztof,
Sorry about that — my previous response might not have made it to the list/thread. Replying here to address your concerns before sending v3.
- DM_DISC reply after LLCP_CLOSED
This is not a new behavior introduced by my change. In the old code, the LLCP_CLOSED branch did release_sock() and nfc_llcp_sock_put(), but it did not return/goto, so execution continued and still reached nfc_llcp_send_dm(..., LLCP_DM_DISC) afterwards. The disc patch only removes the redundant CLOSED-branch cleanup so release_sock()/nfc_llcp_sock_put() are performed exactly once via the common exit path, while keeping the existing DM_DISC reply behavior.
I understand that you did not change the flow. I did not claim you did. I ask why do you think your code is correct.
Do not top post and do not send new versions while the discussion is still going.
- Initial refcount / double free concern
nfc_llcp_recv_disc()/recv_hdlc() take an extra reference via nfc_llcp_sock_get() (sock_hold()). The issue is the mismatched put/unlock: the CLOSED branch drops the reference and releases the lock, and then the common exit path does the same again. This is a refcount/locking imbalance regardless of whether it immediately frees the object, and it may become a UAF depending on timing/refcounting.
You did not really address the problem. The refcnt has imbalance only if you assume initial refcnt was 0.
Best regards, Krzysztof