Re: [PATCH net v3] nfc: llcp: bound SNL TLV parsing to the skb and add length checks

On 04/06/2026 17:22, Doruk Tan Ozturk wrote:
> nfc_llcp_recv_snl() walked the SNL TLV list using a u16 offset/length
> pair derived from skb->len, without bounding reads to the actual skb
> data. Three problems followed:
> 
>    - For a short frame (skb->len < LLCP_HEADER_SIZE), tlv_len underflowed.
>    - The per-TLV header (type, length) was read without checking that two
>      bytes remained.
>    - A declared TLV length could run past the end of the buffer, and an
>      SDREQ with length == 0 made "service_name_len = length - 1" underflow
>      (size_t), driving an out-of-bounds read in the following strncmp() /
>      nfc_llcp_sock_from_sn(). The SDRES case likewise read tlv[2]/tlv[3]
>      without a length check.
> 
> A nearby NFC device can reach this without authentication; LLCP link
> activation happens automatically after NFC-DEP.
> 
> Walk the TLV list by pointer, bounded by skb_tail_pointer() over the
> linear skb data, and validate each TLV declared length before use. Add
> explicit length checks for SDREQ (>= 1) and SDRES (exactly 2).
> 
> Found by 0sec automated security-research tooling (https://0sec.ai).

This version looks much better!

Since you fixing existing issue, adding Fixes tag and relevant commit which 
introduced the problem would be good too.

Thank you for working on it.
David

> 
> Signed-off-by: Doruk Tan Ozturk <[email protected]>
> ---
> v3 (review cleanups, no functional change to the fix):
>   - Comment that only the linear part of the skb is parsed (David Laight).
>   - Use int for service_name_len and print the bounded service name
>     directly with %.*s; drop the min_t()/cast (David Laight).
>   - Require SDRES length to be exactly 2, not just >= 2 (David Laight).
> 
> v2: https://lore.kernel.org/netdev/[email protected]/
>   - Walk by pointer bounded on skb_tail_pointer(); drop the 16-bit
>     offset/tlv_len math and fix the short-frame underflow (David Laight).
>   - Add an SDRES length check alongside SDREQ length >= 1 (David Laight).
>   - Bound the SDREQ service-name pr_debug to the field length.
>   - Rebased onto linux-nfc for-next (David Heidelberg).
> 
>   net/nfc/llcp_core.c | 29 +++++++++++++++++++++--------
>   1 file changed, 21 insertions(+), 8 deletions(-)
> 

[...]