* Re: [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui()
2026-09-16 7:34 [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui() Wentao Liang
@ 2026-09-16 8:37 ` Xuanqiang Luo
2026-09-20 7:48 ` netdev-bot+sashiko
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Xuanqiang Luo @ 2026-09-16 8:37 UTC (permalink / raw)
To: Wentao Liang
Cc: david+nfc, edumazet, horms, kuba, linux-kernel, netdev,
oe-linux-nfc, pabeni, sameo, stable, davem
在 2026/9/16 15:34, Wentao Liang 写道:
> In nfc_llcp_recv_ui(), nfc_llcp_sock_get() is called to find the bound
> socket. If the socket type is not SOCK_DGRAM, the function returns
> directly without releasing the socket reference via nfc_llcp_sock_put(),
> resulting in a reference count leak.
>
> Add nfc_llcp_sock_put() when the socket type is not SOCK_DGRAM.
>
> Fixes: 968272bf0087 ("NFC: Handle LLCP UI frames")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> net/nfc/llcp_core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
> index db5bc6a878dd..5813c80ca593 100644
> --- a/net/nfc/llcp_core.c
> +++ b/net/nfc/llcp_core.c
> @@ -888,8 +888,12 @@ static void nfc_llcp_recv_ui(struct nfc_llcp_local *local,
>
> /* We're looking for a bound socket, not a client one */
> llcp_sock = nfc_llcp_sock_get(local, dsap, LLCP_SAP_SDP);
> - if (llcp_sock == NULL || llcp_sock->sk.sk_type != SOCK_DGRAM)
> + if (!llcp_sock)
> return;
> + if (llcp_sock->sk.sk_type != SOCK_DGRAM) {
> + nfc_llcp_sock_put(llcp_sock);
> + return;
> + }
>
> /* There is no sequence with UI frames */
> skb_pull(skb, LLCP_HEADER_SIZE);
Reviewed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Please use the [PATCH net] subject prefix for fixes targeting the net
tree. See the netdev submission guidelines:
https://docs.kernel.org/process/maintainer-netdev.html#networking-subsystem-netdev
Thanks,
Xuanqiang
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui()
2026-09-16 7:34 [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui() Wentao Liang
2026-09-16 8:37 ` Xuanqiang Luo
@ 2026-09-20 7:48 ` netdev-bot+sashiko
2026-09-21 15:11 ` krzk
2026-09-21 15:12 ` krzk
3 siblings, 0 replies; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-09-20 7:48 UTC (permalink / raw)
To: vulab
Cc: davem, david+nfc, edumazet, horms, kuba, linux-kernel, netdev,
oe-linux-nfc, pabeni, sameo, stable
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 (1) · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Incomplete fix: the identical nfc_llcp_sock_get() reference leak
remains at the sibling call site in nfc_llcp_recv_connect()…
Pre-existing issues:
- [Critical] Post-handoff SKB access / use-after-free race in
nfc_llcp_recv_ui() (net/nfc/llcp_core.c:902-907): the extra SKB…
--
nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui()
This splits the combined NULL/socket-type test in nfc_llcp_recv_ui() so that
the reference taken by nfc_llcp_sock_get() is dropped with
nfc_llcp_sock_put() when the socket is not a SOCK_DGRAM socket. Previously
that branch returned with the reference still held.
> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
> index cac1b5487064d..f83196e49d2a6 100644
> --- a/net/nfc/llcp_core.c
> +++ b/net/nfc/llcp_core.c
> @@ -890,8 +890,12 @@ static void nfc_llcp_recv_ui(struct nfc_llcp_local *local,
>
> /* We're looking for a bound socket, not a client one */
> llcp_sock = nfc_llcp_sock_get(local, dsap, LLCP_SAP_SDP);
> - if (llcp_sock == NULL || llcp_sock->sk.sk_type != SOCK_DGRAM)
> + if (!llcp_sock)
> return;
> + if (llcp_sock->sk.sk_type != SOCK_DGRAM) {
> + nfc_llcp_sock_put(llcp_sock);
> + return;
> + }
[Severity: High]
Does the same nfc_llcp_sock_get() reference get leaked in
nfc_llcp_recv_connect(), which is a few lines below this in the same file?
It has the identical condition shape that is being split apart here:
net/nfc/llcp_core.c:nfc_llcp_recv_connect() {
if (dsap != LLCP_SAP_SDP) {
sock = nfc_llcp_sock_get(local, dsap, LLCP_SAP_SDP);
if (sock == NULL || sock->sk.sk_state != LLCP_LISTEN) {
reason = LLCP_DM_NOBOUND;
goto fail;
}
...
fail:
/* Send DM */
nfc_llcp_send_dm(local, dsap, ssap, reason);
}
The fail: label only sends the DM PDU and returns, so when sock is non-NULL
but sk_state != LLCP_LISTEN, the reference taken by
nfc_llcp_sock_get()'s sock_hold(&llcp_sock->sk) is never released. Every
other exit of that function (acceptq full, LLCP_SAP_MAX, nfc_llcp_sock_alloc
failure, nfc_llcp_local_get failure, and the success path) does call
sock_put(&sock->sk).
Is that branch reachable from a remote peer? nfc_llcp_sock_alloc() sets
llcp_sock->ssap = 0;
llcp_sock->dsap = LLCP_SAP_SDP;
and llcp_sock_bind() publishes the socket with
nfc_llcp_sock_link(&local->sockets, sk);
...
sk->sk_state = LLCP_BOUND;
so a bound-but-not-listening socket (any bound SOCK_DGRAM socket, or a stream
socket bound before listen()) matches the lookup key used by
nfc_llcp_recv_connect(), and a CONNECT PDU aimed at its ssap would leak one
sk_refcnt per frame received. Since the socket would then never reach
llcp_sock_destruct(), does the nfc device reference that destructor releases
leak as well? Would it make sense to fix both call sites in one patch?
>
> /* There is no sequence with UI frames */
> skb_pull(skb, LLCP_HEADER_SIZE);
[Severity: Critical]
This isn't a bug introduced by this patch, but while looking at
nfc_llcp_recv_ui() the ordering of the skb reference just after the quoted
hunk looks racy:
net/nfc/llcp_core.c:nfc_llcp_recv_ui() {
skb_pull(skb, LLCP_HEADER_SIZE);
if (!sock_queue_rcv_skb(&llcp_sock->sk, skb)) {
/*
* UI frames will be freed from the socket layer, so we
* need to keep them alive until someone receives them.
*/
skb_get(skb);
...
}
Can the skb be freed before skb_get() runs? sock_queue_rcv_skb() links the
buffer into sk->sk_receive_queue, drops sk_receive_queue.lock and then calls
sk->sk_data_ready(), so at that point skb->users is still 1 and the buffer is
already visible to a reader on another cpu:
net/nfc/llcp_sock.c:llcp_sock_recvmsg() {
skb = skb_recv_datagram(sk, flags, &err);
...
kfree_skb(skb);
}
If that reader dequeues and frees the buffer first, does skb_get() then do
refcount_inc() on freed memory?
And would nfc_llcp_rx_work() free the same buffer a second time afterwards?
net/nfc/llcp_core.c:nfc_llcp_rx_work() {
...
kfree_skb(local->rx_pending);
local->rx_pending = NULL;
}
No lock appears to span the handoff here: nfc_llcp_recv_ui() does not hold
the socket lock, the queue spinlock is released inside
__sock_queue_rcv_skb() before the wakeup, and the nfc_llcp_sock_get()
reference only keeps the socket alive, not the skb. Would taking the
reference before sock_queue_rcv_skb() and dropping it when queueing fails be
the safer order?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916073424.1971784-1-vulab%40iscas.ac.cn
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui()
2026-09-16 7:34 [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui() Wentao Liang
2026-09-16 8:37 ` Xuanqiang Luo
2026-09-20 7:48 ` netdev-bot+sashiko
@ 2026-09-21 15:11 ` krzk
2026-09-21 15:12 ` krzk
3 siblings, 0 replies; 5+ messages in thread
From: krzk @ 2026-09-21 15:11 UTC (permalink / raw)
To: Wentao Liang
Cc: oe-linux-nfc, davem, kuba, linux-kernel, stable, edumazet,
netdev, pabeni, horms, david+nfc, sameo
On Wed, 16 Sep 2026 07:34:24 +0000, Wentao Liang wrote:
> In nfc_llcp_recv_ui(), nfc_llcp_sock_get() is called to find the bound
> socket. If the socket type is not SOCK_DGRAM, the function returns
> directly without releasing the socket reference via nfc_llcp_sock_put(),
> resulting in a reference count leak.
>
> Add nfc_llcp_sock_put() when the socket type is not SOCK_DGRAM.
>
> Fixes: 968272bf0087 ("NFC: Handle LLCP UI frames")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> net/nfc/llcp_core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem, how to document usage of LLM and how what you should not
do if this was posted in a good faith.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui()
2026-09-16 7:34 [PATCH] nfc: llcp: Fix socket reference leak in nfc_llcp_recv_ui() Wentao Liang
` (2 preceding siblings ...)
2026-09-21 15:11 ` krzk
@ 2026-09-21 15:12 ` krzk
3 siblings, 0 replies; 5+ messages in thread
From: krzk @ 2026-09-21 15:12 UTC (permalink / raw)
To: Wentao Liang
Cc: horms, netdev, pabeni, edumazet, linux-kernel, stable, david+nfc,
davem, kuba, sameo, oe-linux-nfc
On Wed, 16 Sep 2026 07:34:24 +0000, Wentao Liang wrote:
> In nfc_llcp_recv_ui(), nfc_llcp_sock_get() is called to find the bound
> socket. If the socket type is not SOCK_DGRAM, the function returns
> directly without releasing the socket reference via nfc_llcp_sock_put(),
> resulting in a reference count leak.
>
> Add nfc_llcp_sock_put() when the socket type is not SOCK_DGRAM.
>
> Fixes: 968272bf0087 ("NFC: Handle LLCP UI frames")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> net/nfc/llcp_core.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem, how to document usage of LLM and how what you should not
do if this was posted in a good faith.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread