All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Doruk Tan Ozturk <doruk@0sec.ai>
Cc: david@ixit.cz, vadim.fedorenko@linux.dev, horms@kernel.org,
	oe-linux-nfc@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH net v2] nfc: llcp: reject PDUs shorter than the LLCP header
Date: Tue, 14 Jul 2026 21:53:02 +0100	[thread overview]
Message-ID: <20260714215302.75d8bf72@pumpkin> (raw)
In-Reply-To: <20260714164629.75051-1-doruk@0sec.ai>

On Tue, 14 Jul 2026 18:46:29 +0200
Doruk Tan Ozturk <doruk@0sec.ai> wrote:

> > Is there a similar problem with non-linear skb?
> > Maybe they can't get into this code, but who knows what can happen
> > with unusual configs.  
> 
> Good question. Today every skb that reaches __nfc_llcp_recv() is
> linear: the target path (nci_rx_data_packet -> nci_add_rx_data_frag ->
> nfc_tm_data_received) and the initiator path (nfc_data_exchange ->
> nfc_llcp_recv) both build the frame with alloc_skb()/nci_skb_alloc()
> plus skb_put()/skb_put_data(), and NCI reassembly uses skb_cow_head()
> and skb_push() into the linear area. Nothing on the NFC receive side
> attaches page frags or a frag_list, so skb->len == skb_headlen() and the
> v2 skb->len test was in fact sufficient for the in-tree drivers.
> 
> But relying on that is fragile: the parser reads the header out of the
> linear area (pdu->data[0]/data[1]) while skb->len is the total length,
> so a non-linear skb with a short linear head would slip past a skb->len
> test and still over-read the linear buffer. pskb_may_pull() is the
> right guard here -- it also covers the non-linear case, and it matches
> how the sibling NCI and HCI receive paths already validate their
> headers.
> 
> I will send a v3 that uses:
> 
> 	if (!pskb_may_pull(skb, LLCP_HEADER_SIZE)) {
> 		kfree_skb(skb);
> 		return;
> 	}
> 
> That is strictly stronger than the v2 check and does not reject any
> valid frame -- pskb_may_pull() pulls the two header bytes into the
> linear area when needed.

Does that help any code further down the rx path?
It might be better to drop non-linear packets with a comment that
they aren't expected to happen.

	David


> 
> Thanks,
> Doruk


      reply	other threads:[~2026-07-14 20:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-13 15:58 [PATCH net v2] nfc: llcp: reject PDUs shorter than the LLCP header Doruk Tan Ozturk
2026-07-13 21:15 ` David Laight
2026-07-14 16:46   ` Doruk Tan Ozturk
2026-07-14 20:53     ` David Laight [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=20260714215302.75d8bf72@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=david@ixit.cz \
    --cc=doruk@0sec.ai \
    --cc=horms@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oe-linux-nfc@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=vadim.fedorenko@linux.dev \
    /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.