All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
To: "Doruk (0sec)" <doruk@0sec.ai>
Cc: david@ixit.cz, oe-linux-nfc@lists.linux.dev,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH net] nfc: llcp: reject PDUs shorter than the LLCP header
Date: Sun, 12 Jul 2026 23:14:44 +0100	[thread overview]
Message-ID: <f4b2b659-036b-49f7-8f6b-5f173a1eb380@linux.dev> (raw)
In-Reply-To: <CAPdMp1p=72WXe-8w5Y9viBuB6YvZWZomNc9xQzXEg-qe1WVN0A@mail.gmail.com>

On 12/07/2026 17:02, Doruk (0sec) wrote:
> Hi Vadim
> 
> this was reproduced from userspace on unmodified
> linux-next (bee763d5f341) without RF hardware.
> 
> It's the peer-RX path, not a local command skb:
> 
> virtual_ncidev_write (peer NCI DATA) -> nci_rx_data_packet
> -> nfc_tm_data_received -> nfc_llcp_data_received
> -> rx_work -> nfc_llcp_rx_skb -> nfc_llcp_recv_connect

Ok, fair, but in this case it's better to check skb->len in
nfc_llcp_data_received - no need to setup a worker when skb is not
correct.

> 
> Bring the LLCP link up via a normal NFC-DEP activation, then send
> one NCI DATA packet with a 1-byte CONNECT PDU. skb->len - 2 wraps
> to 0xffffffff and the TLV walk runs off the end:
> 
> BUG: KFENCE: out-of-bounds read in nfc_llcp_recv_connect+0x9f6/0xf80
> nfc_llcp_recv_connect+0x9f6 -> nfc_llcp_rx_work -> process_one_work
> read 4219B past a 704B skbuff_small_head from virtual_ncidev_write
> R14: 00000000ffffffff (wrapped tlv_array_len)
> 
> With the guard: rx_skb runs for all 600 short PDUs, recv_connect
> reached 0 times, 0 reports.
> 
> The bound stays "<", not "<=" -- a header-only SYMM/DISC/DM is
> exactly 2 bytes and must still dispatch; AGF uses "<=" only
> because an AGF frame must also carry a sub-PDU. I'll drop the
> "same guard as AGF" line from the commit message.
> 
> Instantiating /dev/virtual_nci needs privilege, but that's just
> the syzbot transport; the 1-byte CONNECT is what a remote NFC-DEP
> peer emits, and the DEP layer imposes no minimum LLCP length.
> Impact is a proximity OOB read (DoS).
> 
> I can send the full reproducer if you'd like.
> 
> best
> Doruk
> 
> On Sun, Jul 12, 2026 02:01 PM, Vadim Fedorenko
> <vadim.fedorenko@linux.dev> wrote:
>>
>> On 11/07/2026 08:27, Doruk Tan Ozturk wrote:
>>> nfc_llcp_rx_skb() reads the two-byte LLCP header (DSAP/SSAP/PTYPE) and
>>> dispatches by PDU type; several handlers then derive a TLV-array length as
>>> skb->len - LLCP_HEADER_SIZE. Neither nfc_llcp_rx_skb() nor its callers
>>> guarantee the frame is at least LLCP_HEADER_SIZE bytes, and a sub-header
>>
>> that's not correct. there are 2 ways to get to nfc_llcp_rx_skb() - via
>> nfc_llcp_recv_agf() or through commands/locally generated skbs. The
>> first one checks against LLCP_HEADER_SIZE, while latter one creates skb
>> payload with correct LLCP header size. Do you have a reproducer to
>> trigger the issue?
>>
>>
>>> PDU does reach it: digital_in_recv_dep_res() and digital_tg_recv_dep_req()
>>> strip the DEP header with skb_pull() after only checking the DEP header
>>> size, so a DEP I-PDU carrying a 0- or 1-byte LLCP payload is handed up as
>>> a sub-2-byte skb.
>>>
>>> For a CONNECT or CC PDU, nfc_llcp_recv_connect() and nfc_llcp_recv_cc()
>>> then pass skb->len - LLCP_HEADER_SIZE to nfc_llcp_parse_connection_tlv().
>>> For skb->len < 2 that subtraction underflows: truncated into the u16
>>> tlv_array_len parameter it becomes ~0xFFFE, and for a CONNECT to the SDP
>>> SAP, nfc_llcp_connect_sn() uses a size_t and underflows to SIZE_MAX. The
>>> TLV parsers bound their walk relative to that length, so they read far
>>> past the end of the skb.
>>>
>>> The aggregated-frame path (nfc_llcp_recv_agf()) already drops sub-PDUs
>>> shorter than the header. Apply the same guard once, in the dispatcher, so
>>
>> that not exactly correct, it drops skbs which are shorter or equal to
>> the header, the check added in this patch is not correct then.
>>
>>> every PDU type is covered.
>>>
>>> Found by 0sec (https://0sec.ai) using automated source analysis; the
>>> missing guard is evident from source. Compile-tested.
>>>
>>> Fixes: d646960f7986 ("NFC: Initial LLCP support")
>>> Cc: stable@vger.kernel.org
>>> Assisted-by: 0sec:claude-opus-4-8
>>> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
>>> ---
>>>    net/nfc/llcp_core.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
>>> index aed5fe1afef0..e3b3077e0e83 100644
>>> --- a/net/nfc/llcp_core.c
>>> +++ b/net/nfc/llcp_core.c
>>> @@ -1481,6 +1481,9 @@ static void nfc_llcp_rx_skb(struct nfc_llcp_local *local, struct sk_buff *skb)
>>>    {
>>>        u8 dsap, ssap, ptype;
>>>
>>> +     if (skb->len < LLCP_HEADER_SIZE)
>>> +             return;
>>> +
>>>        ptype = nfc_llcp_ptype(skb);
>>>        dsap = nfc_llcp_dsap(skb);
>>>        ssap = nfc_llcp_ssap(skb);
>>


      reply	other threads:[~2026-07-12 22:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-11  7:27 [PATCH net] nfc: llcp: reject PDUs shorter than the LLCP header Doruk Tan Ozturk
2026-07-12 12:00 ` Vadim Fedorenko
2026-07-12 16:02   ` Doruk (0sec)
2026-07-12 22:14     ` Vadim Fedorenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f4b2b659-036b-49f7-8f6b-5f173a1eb380@linux.dev \
    --to=vadim.fedorenko@linux.dev \
    --cc=david@ixit.cz \
    --cc=doruk@0sec.ai \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oe-linux-nfc@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.