Re: [PATCH net] nfc: llcp: reject PDUs shorter than the LLCP header

Vadim Fedorenko <[email protected]> Sun, 12 Jul 2026 23:14:44 +0100
Newsgroups dev.linux.lists.oe-linux-nfc,org.kernel.vger.linux-kernel,org.kernel.vger.netdev,org.kernel.vger.stable
Message-ID <[email protected]>
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
> <[email protected]> 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: [email protected]
>>> Assisted-by: 0sec:claude-opus-4-8
>>> Signed-off-by: Doruk Tan Ozturk <[email protected]>
>>> ---
>>>    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);
>>