Re: [PATCH] nfc: llcp: fix u8 offset truncation in LLCP TLV parsers

[Simon Horman <horms@kernel.org>]

On Sun, Apr 05, 2026 at 12:59:38PM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() declare
> 'offset' as u8, but compare it against a u16 tlv_array_len:
> 
>     u8 type, length, offset = 0;
>     while (offset < tlv_array_len) {   /* tlv_array_len is u16 */
>         ...
>         offset += length + 2;          /* wraps at 256 */
>         tlv    += length + 2;
>     }
> 
> When tlv_array_len > 255 -- possible in nfc_llcp_parse_connection_tlv()
> when the peer has negotiated MIUX = 0x7FF (MIU = 2175 bytes), so that
> a CONNECT PDU can carry a TLV array of up to 2173 bytes -- the u8
> offset wraps back below tlv_array_len after every 128 zero-length TLV
> entries and the loop never terminates.  The 'tlv' pointer meanwhile
> advances without bound into adjacent kernel heap, causing:
> 
>   * an OOB read of kernel heap content past the skb end;
>   * a kernel page fault / oops once 'tlv' leaves mapped memory.

Is the more general explanation of this problem that the length of packet
data is used as the tlv_array_len parameter, and that length is not
verified to be within the expected bound?

I am also concerned that the packet data needs to be pulled
before it can be safely accessed.

> 
> This is reachable from any NFC P2P peer device within ~4 cm without
> requiring compromised NFCC firmware.

I think some further explanation is warranted here if that is the case.

> Fix: promote 'offset' from u8 to u16 in both parsers, matching the
> type of their tlv_array_len parameter.
> 
> nfc_llcp_parse_gb_tlv() takes GB bytes from the ATR_RES (max 44 bytes),
> so the wrap cannot occur in practice there.  Change it anyway for
> correctness and to prevent copy-paste reintroduction.

In the case of both nfc_llcp_parse_gb_tlv() it seems to me that wrap-around
can occur because the value of length, which is used to increment the value
of offset, is read from unverified packet data. Is the data validate
somewhere, or known to be correct?

If so, I expect this can also result in an overrun of tlv.

And I think the same problem exists in nfc_llcp_parse_gb_tlv().

> 
> Cc: stable@vger.kernel.org

A fixes tag is needed here.

> Signed-off-by: Lekë Hapçiu <framemain@outlook.com>

The patch should be targeted at net-next like this:

Subject: [PATCH net] ...

And please group together related patches - I see several for nfc -
in a patchset.

More on the Netdev development process can be found here
https://docs.kernel.org/process/maintainer-netdev.html

-- 
pw-bot: changes-requested