[PATCH net] nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

[PATCH net] nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

nfc_llcp_allocate_snl() sums the requested TLV payload length into a u16
variable before passing it to alloc_skb(), but its tlv_length argument
is a size_t and callers pass a length bounded only by the size of the
netlink message that produced it.

nfc_genl_llc_sdreq() walks the nested NFC_ATTR_LLC_SDP attributes of a
GENL_ADMIN_PERM request, builds one SDREQ TLV per attribute and sums
their lengths into the size_t tlvs_len. Each URI is capped at
U8_MAX - 4 bytes by nfc_sdp_genl_policy, so every TLV fits in its u8
tlv_len, but nothing caps the number of attributes, so tlvs_len can
exceed U16_MAX.

When it does, the u16 accumulator wraps and alloc_skb() returns a
buffer that is far too small. nfc_llcp_send_snl_sdreq() then calls
skb_put_data() once per TLV and writes all tlvs_len bytes into it,
which trips skb_over_panic() and panics the machine.

Use size_t for the accumulated size so that it can no longer wrap
before the allocation. alloc_skb() takes an unsigned int, which is
still wide enough for any length a netlink message can carry.

Fixes: e0ae7bac06cc ("NFC: llcp: Service Name Lookup SDRES aggregation")
Reported-by: Guikun Yang <yangguikun19@qq.com>
Link: https://lore.kernel.org/netdev/tencent_634C55A23C7CCCA3985283BFED46A69C9E09@qq.com/
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 net/nfc/llcp_commands.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 80a00938c869..0a3d66e6cd59 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -542,7 +542,7 @@ static struct sk_buff *nfc_llcp_allocate_snl(struct nfc_llcp_local *local,
 {
 	struct sk_buff *skb;
 	struct nfc_dev *dev;
-	u16 size = 0;
+	size_t size = 0;
 
 	if (local == NULL)
 		return ERR_PTR(-ENODEV);
-- 
2.25.1

Re: [PATCH net] nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

[Linkui Xiao]

Hi,

Thanks for the reminder.

On 2026/11/11 18:02, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - Whether the issue was actually triggered, or is only theoretical
>     (e.g. found by LLM and not proven at runtime). If it was triggered
>     please include the symptoms, like the stack trace or kernel messages.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

This patch is a fix for the issue previously reported here:

Link: 
https://lore.kernel.org/netdev/tencent_634C55A23C7CCCA3985283BFED46A69C9E09@qq.com/

The commit message already includes:

Reported-by: Guikun Yang <yangguikun19@qq.com>
Link: 
https://lore.kernel.org/netdev/tencent_634C55A23C7CCCA3985283BFED46A69C9E09@qq.com/

To clarify: I did not reproduce this issue at runtime in my testing.
The problem was identified by code review/static analysis of
nfc_llcp_allocate_snl(), where the SNL PDU size can be truncated.
So this is a code-review/static-analysis finding rather than a
runtime-triggered crash in my environment.

Please let me know if further information is needed. As requested,
I will not repost the series just to add this clarification.

Thanks,
Linkui

Re: [PATCH net] nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 1 · 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).

- [Medium] The commit message names the wrong trigger and leaves out the
  real one.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch
  makes it easier to reach.

--

nfc: llcp: fix SNL PDU size truncation in nfc_llcp_allocate_snl()

This commit changes the size accumulator in nfc_llcp_allocate_snl() from
u16 to size_t, so the SNL PDU allocation size can no longer wrap before
alloc_skb(). The commit message says the overflow comes from
nfc_genl_llc_sdreq() building many SDREQ TLVs out of a netlink request.

Link: https://lore.kernel.org/netdev/tencent_634C55A23C7CCCA3985283BFED46A69C9E09@qq.com/

> nfc_genl_llc_sdreq() walks the nested NFC_ATTR_LLC_SDP attributes of a
> GENL_ADMIN_PERM request, builds one SDREQ TLV per attribute and sums
> their lengths into the size_t tlvs_len. Each URI is capped at
> U8_MAX - 4 bytes by nfc_sdp_genl_policy, so every TLV fits in its u8
> tlv_len, but nothing caps the number of attributes, so tlvs_len can
> exceed U16_MAX.

[Severity: Medium]

Can this netlink path actually wrap the u16?

nfc_genl_llc_sdreq() only walks a single attribute:

    nla_for_each_nested(attr, info->attrs[NFC_ATTR_LLC_SDP], rem) {

NFC_ATTR_LLC_SDP is one NLA_NESTED attribute, and its nla_len is a
__u16, so the nested payload is at most 65531 bytes.

Each entry costs at least 8 bytes of nest and URI attribute headers plus
NLA_ALIGN(uri_len), and it produces a TLV of only uri_len + 3 bytes. With
uri_len = 251 that comes to about 64014 TLV bytes at most. The
LLCP_HEADER_SIZE, NFC_HEADER_SIZE, tx_headroom and tx_tailroom that
nfc_llcp_allocate_snl() adds are small for the in-tree drivers.

The path that does look able to wrap is the remote SDRES one:

nfc_llcp_recv_snl()
    nfc_llcp_build_sdres_tlv()   /* 4 bytes per 3-byte SDREQ TLV */
    nfc_llcp_send_snl_sdres(local, &llc_sdres_list, sdres_tlvs_len)
        nfc_llcp_allocate_snl()

A peer in NFC range can send one SNL PDU packed with 3-byte SDREQ TLVs
(type, len=1, tid). sdres_tlvs_len then grows to about 4/3 of the frame
size.

Neither nci_add_rx_data_frag() (via skb_cow_head()) nor
digital_recv_dep_data_gather() (via skb_copy_expand()) caps the
reassembled frame. An SNL PDU of about 49 KB would therefore wrap the old
u16 size and hit skb_over_panic() in nfc_llcp_send_snl_sdres().

The code change fixes both senders, since they share
nfc_llcp_allocate_snl(). Could the commit message describe the SDRES path
that an unprivileged remote peer can trigger, instead of the
GENL_ADMIN_PERM netlink path? As written, it reads as if the bug needs
CAP_NET_ADMIN to trigger.

> diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
> index 80a00938c8696..0a3d66e6cd59f 100644
> --- a/net/nfc/llcp_commands.c
> +++ b/net/nfc/llcp_commands.c
> @@ -542,7 +542,7 @@ static struct sk_buff *nfc_llcp_allocate_snl(struct nfc_llcp_local *local,
>  {
>  	struct sk_buff *skb;
>  	struct nfc_dev *dev;
> -	u16 size = 0;
> +	size_t size = 0;

[Severity: High]

This is a pre-existing issue, but now that size no longer wraps, does
nfc_llcp_recv_snl() leak the llc_sdres_list entries when alloc_skb() fails
here?

nfc_llcp_send_snl_sdres() returns before the loop that frees the entries:

net/nfc/llcp_commands.c:nfc_llcp_send_snl_sdres() {
	skb = nfc_llcp_allocate_snl(local, tlvs_len);
	if (IS_ERR(skb))
		return PTR_ERR(skb);

	hlist_for_each_entry_safe(sdp, n, tlv_list, node) {
		skb_put_data(skb, sdp->tlv, sdp->tlv_len);

		hlist_del(&sdp->node);

		nfc_llcp_free_sdp_tlv(sdp);
	}
    ...
}

nfc_llcp_recv_snl() ignores the return value, and the list head is on its
stack:

net/nfc/llcp_core.c:nfc_llcp_recv_snl() {
	HLIST_HEAD(llc_sdres_list);
    ...
	if (!hlist_empty(&llc_sdres_list))
		nfc_llcp_send_snl_sdres(local, &llc_sdres_list, sdres_tlvs_len);
}

Each SDREQ TLV from the peer leaves behind a struct nfc_llcp_sdp_tlv and
its 4-byte tlv buffer. That is about 72 bytes for every 3 bytes on the
wire.

Before this change, a frame that needed more than 64 KiB wrapped size and
panicked. Now alloc_skb() gets the full size, about 4/3 of the frame.

That allocation can fail for costly orders under fragmentation. It always
fails once the request is above MAX_PAGE_ORDER: on x86-64 with 4 KiB
pages, that is an SDRES over 4 MiB, from an SNL PDU of about 3 MiB.

Frame reassembly has no cap, so one such frame would leak around 1M
entries, roughly 72 MB. Repeated frames could drive the system to OOM.

The same leak happens when local->dev is NULL and nfc_llcp_allocate_snl()
returns -ENODEV.

Should the tlv_list entries be freed on this error path, either in
nfc_llcp_send_snl_sdres() or in its caller?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111095907.3366028-1-xiaolinkui%40126.com