[PATCH net v5 0/3] nfc: fix remaining OOB bugs in NCI/LLCP parsing

[PATCH net v5 0/3] nfc: fix remaining OOB bugs in NCI/LLCP parsing

[Lekë Hapçiu]

Rebased against David's linux-nfc for-linus tree [1], as requested.

This was originally a 5-patch series. Two of the five (the
parse_gb_tlv()/parse_connection_tlv() offset-wrap fix and the
nfc_llcp_recv_snl() TLV bounds fix) have since been fixed independently
by other contributors already merged into for-linus:

  d8bd2dedbde5 ("nfc: llcp: fix OOB read and u8 offset wrap in TLV parsers")
  27256cdb290e ("nfc: llcp: bound SNL TLV parsing to the skb and add length checks")

Those two are dropped from this series to avoid duplicating work. The
remaining three patches are unchanged in substance from v4, just
rebased and renumbered:

  1/3 (was 1/5) - nci_store_general_bytes_nfc_dep() u8 underflow
  2/3 (was 4/5) - nfc_llcp_recv_dm() OOB read of the reason byte
  3/3 (was 5/5) - nfc_llcp_connect_sn() TLV parsing OOB

All three still reproduce against current for-linus (verified against
1671b8fb7300 before rebase). checkpatch --strict is clean on all three.

[1] https://codeberg.org/linux-nfc/linux.git for-linus

Lekë Hapçiu (3):
  nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
  nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm
  nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn

 net/nfc/llcp_core.c | 19 +++++++++++++++++--
 net/nfc/nci/ntf.c   |  6 ++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

-- 
2.51.0

[PATCH net v5 1/3] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep

[Lekë Hapçiu]

nci_store_general_bytes_nfc_dep() computes the General Bytes length by
subtracting a fixed header offset from the peer-supplied atr_res_len
(POLL) or atr_req_len (LISTEN) field:

    ndev->remote_gb_len = min_t(__u8,
        atr_res_len - NFC_ATR_RES_GT_OFFSET,   /* offset = 15 */
        NFC_ATR_RES_GB_MAXSIZE);

Both length fields are __u8.  When a malicious NFC-DEP peer sends an
ATR_RES/ATR_REQ whose length is smaller than the fixed offset (< 15
or < 14 respectively), the subtraction wraps:

    atr_res_len = 0  ->  (u8)(0 - 15) = 241
    min_t(__u8, 241, NFC_ATR_RES_GB_MAXSIZE=47) = 47

The subsequent memcpy then reads 47 bytes beyond the valid activation
parameter data into ndev->remote_gb[].  This buffer is later fed to
nfc_llcp_parse_gb_tlv() as a TLV array.

Reject the frame with NCI_STATUS_RF_PROTOCOL_ERROR when the length is
below the required offset.  The existing caller already logs and
continues for other helpers that return a non-OK status from this
switch, so no change is required on the caller side.

Fixes: 767f19ae698e ("NFC: Implement NCI dep_link_up and dep_link_down")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.com>
---
 net/nfc/nci/ntf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index 87f76a29f0ec..1a38f4666e97 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -653,6 +653,9 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev,
 	switch (ntf->activation_rf_tech_and_mode) {
 	case NCI_NFC_A_PASSIVE_POLL_MODE:
 	case NCI_NFC_F_PASSIVE_POLL_MODE:
+		if (ntf->activation_params.poll_nfc_dep.atr_res_len <
+		    NFC_ATR_RES_GT_OFFSET)
+			return NCI_STATUS_RF_PROTOCOL_ERROR;
 		ndev->remote_gb_len = min_t(__u8,
 			(ntf->activation_params.poll_nfc_dep.atr_res_len
 						- NFC_ATR_RES_GT_OFFSET),
@@ -665,6 +668,9 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev,
 
 	case NCI_NFC_A_PASSIVE_LISTEN_MODE:
 	case NCI_NFC_F_PASSIVE_LISTEN_MODE:
+		if (ntf->activation_params.listen_nfc_dep.atr_req_len <
+		    NFC_ATR_REQ_GT_OFFSET)
+			return NCI_STATUS_RF_PROTOCOL_ERROR;
 		ndev->remote_gb_len = min_t(__u8,
 			(ntf->activation_params.listen_nfc_dep.atr_req_len
 						- NFC_ATR_REQ_GT_OFFSET),
-- 
2.51.0

[PATCH net v5 2/3] nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm

[Lekë Hapçiu]

nfc_llcp_recv_dm() reads skb->data[2] (the DM reason byte) without
first verifying that skb->len is at least LLCP_HEADER_SIZE + 1.  A DM
PDU carrying only the 2-byte LLCP header from a rogue peer therefore
triggers a 1-byte OOB read.

Add the minimum-length guard at function entry, matching the pattern
used by nfc_llcp_recv_snl() and nfc_llcp_recv_agf().

Fixes: 5c0560b7a5c6 ("NFC: Handle LLCP Disconnected Mode frames")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.com>
---
 net/nfc/llcp_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index aed5fe1afef0..edec2fd83f79 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1250,6 +1250,11 @@ static void nfc_llcp_recv_dm(struct nfc_llcp_local *local,
 	struct sock *sk;
 	u8 dsap, ssap, reason;
 
+	if (skb->len < LLCP_HEADER_SIZE + 1) {
+		pr_err("Malformed DM PDU\n");
+		return;
+	}
+
 	dsap = nfc_llcp_dsap(skb);
 	ssap = nfc_llcp_ssap(skb);
 	reason = skb->data[2];
-- 
2.51.0

[PATCH net v5 3/3] nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn

[Lekë Hapçiu]

nfc_llcp_connect_sn() walks the TLV array of an LLCP CONNECT PDU
looking for the Service Name TLV, but shares the same class of bugs
as nfc_llcp_recv_snl() / nfc_llcp_parse_gb_tlv():

 1. tlv_array_len = skb->len - LLCP_HEADER_SIZE wraps when skb->len
    is 0 or 1.  The subsequent loop then runs far past the buffer.

 2. The per-iteration guard `offset < tlv_array_len` only proves one
    byte is available, but the body reads both tlv[0] (type) and
    tlv[1] (length).

 3. The peer-supplied `length` field is used to advance `tlv` without
    being checked against the remaining array space, so a crafted
    length walks `tlv` past the buffer.  On the following iteration
    tlv[0]/tlv[1] are read from adjacent memory.

 4. When an LLCP_TLV_SN is found, the function returns &tlv[2] with
    *sn_len = length but without verifying that `length` bytes at
    tlv[2..] are still inside the TLV array.  The caller in
    nfc_llcp_recv_connect() then uses this (pointer, length) pair as
    a service name, so it may read past the PDU.

Fix: reject frames smaller than LLCP_HEADER_SIZE up front; add TLV
header and TLV value guards at the top of each iteration.  The value
guard also ensures that the (&tlv[2], length) pair returned on
LLCP_TLV_SN lies fully inside the TLV array.

Also use LLCP_HEADER_SIZE instead of the magic literal `2` to match
the style of neighbouring LLCP receive paths.

Reported-by: Simon Horman <horms@kernel.org>
Closes: https://lore.kernel.org/netdev/20260417160438.GH31784@horms.kernel.org/
Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.com>
---
 net/nfc/llcp_core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index edec2fd83f79..c4dda8e7cfcf 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -849,12 +849,22 @@ static struct nfc_llcp_sock *nfc_llcp_sock_get_sn(struct nfc_llcp_local *local,
 static const u8 *nfc_llcp_connect_sn(const struct sk_buff *skb, size_t *sn_len)
 {
 	u8 type, length;
-	const u8 *tlv = &skb->data[2];
-	size_t tlv_array_len = skb->len - LLCP_HEADER_SIZE, offset = 0;
+	const u8 *tlv;
+	size_t tlv_array_len, offset = 0;
+
+	if (skb->len < LLCP_HEADER_SIZE)
+		return NULL;
+
+	tlv = &skb->data[LLCP_HEADER_SIZE];
+	tlv_array_len = skb->len - LLCP_HEADER_SIZE;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			break;
 		type = tlv[0];
 		length = tlv[1];
+		if (tlv_array_len - offset - 2 < length)
+			break;
 
 		pr_debug("type 0x%x length %d\n", type, length);
 
-- 
2.51.0

Re: [PATCH net v5 0/3] nfc: fix remaining OOB bugs in NCI/LLCP parsing

[David Heidelberg]

On 16/07/2026 22:35, Lekë Hapçiu wrote:
> Rebased against David's linux-nfc for-linus tree [1], as requested.
> 
> This was originally a 5-patch series. Two of the five (the
> parse_gb_tlv()/parse_connection_tlv() offset-wrap fix and the
> nfc_llcp_recv_snl() TLV bounds fix) have since been fixed independently
> by other contributors already merged into for-linus:
> 
>    d8bd2dedbde5 ("nfc: llcp: fix OOB read and u8 offset wrap in TLV parsers")
>    27256cdb290e ("nfc: llcp: bound SNL TLV parsing to the skb and add length checks")
> 
> Those two are dropped from this series to avoid duplicating work. The
> remaining three patches are unchanged in substance from v4, just
> rebased and renumbered:
> 
>    1/3 (was 1/5) - nci_store_general_bytes_nfc_dep() u8 underflow
>    2/3 (was 4/5) - nfc_llcp_recv_dm() OOB read of the reason byte
>    3/3 (was 5/5) - nfc_llcp_connect_sn() TLV parsing OOB
> 
> All three still reproduce against current for-linus (verified against
> 1671b8fb7300 before rebase). checkpatch --strict is clean on all three.
> 
> [1] https://codeberg.org/linux-nfc/linux.git for-linus
> 
> Lekë Hapçiu (3):
>    nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
>    nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm
>    nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn
> 
>   net/nfc/llcp_core.c | 19 +++++++++++++++++--
>   net/nfc/nci/ntf.c   |  6 ++++++
>   2 files changed, 23 insertions(+), 2 deletions(-)
> 

Hello Lekë,

I'm afraid I won't make you happy here, but after merging the outstanding 
backlog today, I ran into conflicts again with your patch series. I'm very sorry 
about that, but I've been receiving a high number of fixes.

Now that I'm back from the conference, if you send the next revision based on 
the current for-linus / for-next, I believe I'll be able to apply it quickly 
enough to avoid conflicts.

Thank you for your understanding.

David