[PATCH] nfc: llcp: fix u8 offset truncation in LLCP TLV parsers

[PATCH] nfc: llcp: fix u8 offset truncation in LLCP TLV parsers

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() declare
'offset' as u8, but compare it against a u16 tlv_array_len:

    u8 type, length, offset = 0;
    while (offset < tlv_array_len) {   /* tlv_array_len is u16 */
        ...
        offset += length + 2;          /* wraps at 256 */
        tlv    += length + 2;
    }

When tlv_array_len > 255 -- possible in nfc_llcp_parse_connection_tlv()
when the peer has negotiated MIUX = 0x7FF (MIU = 2175 bytes), so that
a CONNECT PDU can carry a TLV array of up to 2173 bytes -- the u8
offset wraps back below tlv_array_len after every 128 zero-length TLV
entries and the loop never terminates.  The 'tlv' pointer meanwhile
advances without bound into adjacent kernel heap, causing:

  * an OOB read of kernel heap content past the skb end;
  * a kernel page fault / oops once 'tlv' leaves mapped memory.

This is reachable from any NFC P2P peer device within ~4 cm without
requiring compromised NFCC firmware.

Fix: promote 'offset' from u8 to u16 in both parsers, matching the
type of their tlv_array_len parameter.

nfc_llcp_parse_gb_tlv() takes GB bytes from the ATR_RES (max 44 bytes),
so the wrap cannot occur in practice there.  Change it anyway for
correctness and to prevent copy-paste reintroduction.

Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/llcp_commands.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 291f26fac..6937dcb3b 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -193,7 +193,8 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 			  const u8 *tlv_array, u16 tlv_array_len)
 {
 	const u8 *tlv = tlv_array;
-	u8 type, length, offset = 0;
+	u8 type, length;
+	u16 offset = 0;
 
 	pr_debug("TLV array length %d\n", tlv_array_len);
 
@@ -243,7 +244,8 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 				  const u8 *tlv_array, u16 tlv_array_len)
 {
 	const u8 *tlv = tlv_array;
-	u8 type, length, offset = 0;
+	u8 type, length;
+	u16 offset = 0;
 
 	pr_debug("TLV array length %d\n", tlv_array_len);
 
-- 
2.51.0

Re: [PATCH] nfc: llcp: fix u8 offset truncation in LLCP TLV parsers

[Simon Horman]

On Sun, Apr 05, 2026 at 12:59:38PM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() declare
> 'offset' as u8, but compare it against a u16 tlv_array_len:
> 
>     u8 type, length, offset = 0;
>     while (offset < tlv_array_len) {   /* tlv_array_len is u16 */
>         ...
>         offset += length + 2;          /* wraps at 256 */
>         tlv    += length + 2;
>     }
> 
> When tlv_array_len > 255 -- possible in nfc_llcp_parse_connection_tlv()
> when the peer has negotiated MIUX = 0x7FF (MIU = 2175 bytes), so that
> a CONNECT PDU can carry a TLV array of up to 2173 bytes -- the u8
> offset wraps back below tlv_array_len after every 128 zero-length TLV
> entries and the loop never terminates.  The 'tlv' pointer meanwhile
> advances without bound into adjacent kernel heap, causing:
> 
>   * an OOB read of kernel heap content past the skb end;
>   * a kernel page fault / oops once 'tlv' leaves mapped memory.

Is the more general explanation of this problem that the length of packet
data is used as the tlv_array_len parameter, and that length is not
verified to be within the expected bound?

I am also concerned that the packet data needs to be pulled
before it can be safely accessed.

> 
> This is reachable from any NFC P2P peer device within ~4 cm without
> requiring compromised NFCC firmware.

I think some further explanation is warranted here if that is the case.

> Fix: promote 'offset' from u8 to u16 in both parsers, matching the
> type of their tlv_array_len parameter.
> 
> nfc_llcp_parse_gb_tlv() takes GB bytes from the ATR_RES (max 44 bytes),
> so the wrap cannot occur in practice there.  Change it anyway for
> correctness and to prevent copy-paste reintroduction.

In the case of both nfc_llcp_parse_gb_tlv() it seems to me that wrap-around
can occur because the value of length, which is used to increment the value
of offset, is read from unverified packet data. Is the data validate
somewhere, or known to be correct?

If so, I expect this can also result in an overrun of tlv.

And I think the same problem exists in nfc_llcp_parse_gb_tlv().

> 
> Cc: stable@vger.kernel.org

A fixes tag is needed here.

> Signed-off-by: Lekë Hapçiu <framemain@outlook.com>

The patch should be targeted at net-next like this:

Subject: [PATCH net] ...

And please group together related patches - I see several for nfc -
in a patchset.

More on the Netdev development process can be found here
https://docs.kernel.org/process/maintainer-netdev.html

-- 
pw-bot: changes-requested

[PATCH net v2 0/3] nfc: fix chained TLV parsing and integer underflow vulnerabilities

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

These three patches fix vulnerabilities in the NFC LLCP and NCI subsystems
that form an exploit chain.  Each bug is independently reachable from an
unauthenticated NFC peer at ~4 cm range; together they create a path from
controlled heap disclosure to heap corruption.

--- Chain summary ---

  [1/3]  nci/ntf.c — nci_store_general_bytes_nfc_dep()
         u8 integer underflow: when the peer's ATR_RES/ATR_REQ length field
         is smaller than NFC_ATR_RES_GT_OFFSET (15) or NFC_ATR_REQ_GT_OFFSET
         (14), the subtraction wraps to a large u8 value.  min_t(__u8, ...)
         clamps to NFC_ATR_RES_GB_MAXSIZE (47), and a 47-byte memcpy reads
         out-of-bounds data into ndev->remote_gb[].  This corrupted buffer
         is subsequently parsed by nfc_llcp_parse_gb_tlv().

  [2/3]  llcp_commands.c — nfc_llcp_parse_gb_tlv() +
                           nfc_llcp_parse_connection_tlv()
         Two bugs in both TLV parsers.  The first, a u8 offset truncation
         causing an infinite loop, was addressed in v1 of this series.
         This version adds the fix identified during review by Simon Horman:
         the `length` byte is read from peer-controlled data with no check
         that the remainder of the array can accommodate `length` more bytes.
         A crafted `length` advances the `tlv` pointer into adjacent kernel
         memory; the next iteration reads tlv[0]/tlv[1] from that location.
         When combined with [1/3], a crafted `length` in the garbage-filled
         remote_gb[] can walk `tlv` past nfc_llcp_local.remote_gb[] and into
         adjacent struct fields, including sdreq_timer.function at ~+176 bytes,
         enabling a kernel pointer disclosure via sock->remote_miu/getsockopt.

  [3/3]  llcp_core.c — nfc_llcp_recv_snl()
         The SNL TLV parsing loop carries the same missing guards as [2/3].
         Additionally: LLCP_TLV_SDREQ accesses tlv[2] and computes
         `service_name_len = length - 1` (u8 underflow to 255 when length==0,
         causing a 255-byte kernel memory scan via strncmp); and
         LLCP_TLV_SDRES accesses tlv[2] and tlv[3] without verifying
         length >= 2.  Unlike the parsers in [2/3], SDREQ/SDRES are processed
         directly without the llcp_tlv_length[] table protection.  A missing
         skb->len guard also allows tlv_len to underflow to ~65534 if
         skb->len < LLCP_HEADER_SIZE.

--- Individual CVSS (AV:A/AC:L/PR:N/UI:N/S:U) ---

  [1/3]  C:H/I:N/A:L  — 6.5
  [2/3]  C:H/I:N/A:L  — 6.5
  [3/3]  C:H/I:N/A:L  — 6.5

--- Chain CVSS ---

  AV:A/AC:H/PR:N/UI:N/S:C/C:H/I:H/A:H — 8.3

  KASLR bypass via [1/3]+[2/3] makes [3/3] reliably exploitable without
  the race-condition timing required against the bugs in isolation.

All patches carry Cc: stable@vger.kernel.org.


Lekë Hapçiu (3):
  nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
  nfc: llcp: add TLV length bounds checks in parse_gb_tlv and
    parse_connection_tlv
  nfc: llcp: fix TLV parsing OOB and length underflow in
    nfc_llcp_recv_snl

 net/nfc/llcp_commands.c | 22 ++++++++++++++++++++++
 net/nfc/llcp_core.c     | 13 +++++++++++++
 net/nfc/nci/ntf.c       | 22 ++++++++++++++--------
 3 files changed, 49 insertions(+), 8 deletions(-)

-- 
2.51.0

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

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

nci_store_general_bytes_nfc_dep() computes the number of General Bytes
to copy from an ATR_RES or ATR_REQ frame by subtracting a fixed header
offset from the peer-supplied length 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 target (POLL mode)
or initiator (LISTEN mode) sends an ATR_RES/ATR_REQ whose length field is
smaller than the fixed offset (< 15 or < 14 respectively), the subtraction
wraps in unsigned u8 arithmetic:

  e.g. atr_res_len = 0 -> (u8)(0 - 15) = 241

min_t(__u8, 241, 47) then yields 47, so the subsequent memcpy reads
47 bytes from beyond the end of the valid activation parameter data into
ndev->remote_gb[].  This buffer is later passed to nfc_llcp_parse_gb_tlv()
as a TLV array, feeding directly into the TLV parser hardened by the
companion patch.

Fix: add an explicit lower-bound check on each length field before the
subtraction.  If the length is smaller than the required offset the frame
is malformed; leave remote_gb_len at zero and skip the memcpy.

Both the POLL (atr_res_len / NFC_ATR_RES_GT_OFFSET = 15) and the LISTEN
(atr_req_len / NFC_ATR_REQ_GT_OFFSET = 14) paths are affected; both are
fixed symmetrically.

Reachability: the ATR_RES is sent by an NFC-DEP target during RF
activation, before any authentication or pairing.  The bug is therefore
reachable from any NFC peer within ~4 cm.

Fixes: a99903ec4566 ("NFC: NCI: Handle Target mode activation")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/nci/ntf.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index c96512bb8..8eb295580 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -631,25 +631,31 @@ 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)
+			break;
 		ndev->remote_gb_len = min_t(__u8,
-			(ntf->activation_params.poll_nfc_dep.atr_res_len
-						- NFC_ATR_RES_GT_OFFSET),
+			ntf->activation_params.poll_nfc_dep.atr_res_len
+						- NFC_ATR_RES_GT_OFFSET,
 			NFC_ATR_RES_GB_MAXSIZE);
 		memcpy(ndev->remote_gb,
-		       (ntf->activation_params.poll_nfc_dep.atr_res
-						+ NFC_ATR_RES_GT_OFFSET),
+		       ntf->activation_params.poll_nfc_dep.atr_res
+						+ NFC_ATR_RES_GT_OFFSET,
 		       ndev->remote_gb_len);
 		break;
 
 	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)
+			break;
 		ndev->remote_gb_len = min_t(__u8,
-			(ntf->activation_params.listen_nfc_dep.atr_req_len
-						- NFC_ATR_REQ_GT_OFFSET),
+			ntf->activation_params.listen_nfc_dep.atr_req_len
+						- NFC_ATR_REQ_GT_OFFSET,
 			NFC_ATR_REQ_GB_MAXSIZE);
 		memcpy(ndev->remote_gb,
-		       (ntf->activation_params.listen_nfc_dep.atr_req
-						+ NFC_ATR_REQ_GT_OFFSET),
+		       ntf->activation_params.listen_nfc_dep.atr_req
+						+ NFC_ATR_REQ_GT_OFFSET,
 		       ndev->remote_gb_len);
 		break;
 
-- 
2.51.0

[PATCH net v2 2/3] nfc: llcp: add TLV length bounds checks in parse_gb_tlv and parse_connection_tlv

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

v1 of this fix promoted `offset` from u8 to u16 in both TLV parsers,
preventing the infinite loop when a connection TLV array exceeds 255 bytes.
During review, Simon Horman identified two additional issues that the u16
promotion alone does not address.

Issue 1 - truncated TLV header:

  The loop guard `offset < tlv_array_len` is not sufficient to guarantee
  that reading tlv[0] (type) and tlv[1] (length) is safe.  When exactly
  one byte remains (offset == tlv_array_len - 1) the loop body reads
  tlv[1] one byte past the end of the array.

Issue 2 - peer-controlled `length` field:

  `length` is read from peer-supplied frame data and is not checked against
  the remaining array space before advancing `tlv` and `offset`:

    offset += length + 2;   /* always */
    tlv    += length + 2;   /* may now point past buffer end */

  A crafted `length` advances `tlv` past the array boundary; the following
  iteration reads tlv[0]/tlv[1] from adjacent kernel memory.

  For nfc_llcp_parse_gb_tlv() this is particularly impactful: its input is
  &local->remote_gb[3], a field within nfc_llcp_local.  A large `length`
  can walk `tlv` into adjacent struct fields including sdreq_timer and
  sdreq_timeout_work which contain kernel function pointers at approximately
  +176 and +216 bytes past remote_gb[].  The parsed `type` byte at those
  positions may match a recognized TLV type causing the parser to store
  bytes from the function pointer into local->remote_miu, which is
  subsequently readable via getsockopt().

Issue 3 - zero-length TLV value:

  The llcp_tlv8() and llcp_tlv16() accessor helpers read tlv[2] and
  tlv[2..3] respectively.  The outer guard guarantees `length` bytes of
  value are available past the two-byte header, but when length == 0 it
  only guarantees offset+2 <= tlv_array_len (non-strict), leaving tlv[2]
  out of bounds.  Per-type minimum-length checks are required before each
  accessor call.  Note: llcp_tlv8/16 additionally validate against the
  llcp_tlv_length[] table, providing a second safety layer; the per-type
  checks here make the rejection explicit and avoid silent zero-defaults.

Fix: add two loop-level guards inside each parsing loop:

  if (tlv_array_len - offset < 2)            /* need type + length */
      break;
  [read type, length]
  if (tlv_array_len - offset - 2 < length)   /* need length value bytes */
      break;

Both subtractions are safe: the loop condition guarantees offset <
tlv_array_len; the first guard then guarantees the difference is >= 2,
making the second subtraction non-negative.

Add per-type minimum-length checks before each accessor call:
  - tlv8-based (VERSION, LTO, OPT, RW): require length >= 1
  - tlv16-based (MIUX, WKS):            require length >= 2

Reachability: nfc_llcp_parse_connection_tlv() is reached on receipt of a
CONNECT or CC PDU before any connection is established.
nfc_llcp_parse_gb_tlv() is reached during ATR_RES processing.  Both are
triggerable from any NFC peer within ~4 cm with no authentication.

Reported-by: Simon Horman <horms@kernel.org>
Fixes: 7a06e586b9bf ("NFC: Move LLCP receiver window value to socket structure")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/llcp_commands.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 6937dcb3b..7cc237a6d 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -202,25 +202,39 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 		return -ENODEV;
 
 	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);
 
 		switch (type) {
 		case LLCP_TLV_VERSION:
+			if (length < 1)
+				break;
 			local->remote_version = llcp_tlv_version(tlv);
 			break;
 		case LLCP_TLV_MIUX:
+			if (length < 2)
+				break;
 			local->remote_miu = llcp_tlv_miux(tlv) + 128;
 			break;
 		case LLCP_TLV_WKS:
+			if (length < 2)
+				break;
 			local->remote_wks = llcp_tlv_wks(tlv);
 			break;
 		case LLCP_TLV_LTO:
+			if (length < 1)
+				break;
 			local->remote_lto = llcp_tlv_lto(tlv) * 10;
 			break;
 		case LLCP_TLV_OPT:
+			if (length < 1)
+				break;
 			local->remote_opt = llcp_tlv_opt(tlv);
 			break;
 		default:
@@ -253,16 +267,24 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 		return -ENOTCONN;
 
 	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);
 
 		switch (type) {
 		case LLCP_TLV_MIUX:
+			if (length < 2)
+				break;
 			sock->remote_miu = llcp_tlv_miux(tlv) + 128;
 			break;
 		case LLCP_TLV_RW:
+			if (length < 1)
+				break;
 			sock->remote_rw = llcp_tlv_rw(tlv);
 			break;
 		case LLCP_TLV_SN:
-- 
2.51.0

[PATCH net v2 3/3] nfc: llcp: fix TLV parsing OOB and length underflow in nfc_llcp_recv_snl

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

nfc_llcp_recv_snl() contains four distinct vulnerabilities.

Issue 1 - missing minimum-length guard on skb:

  nfc_llcp_dsap() and nfc_llcp_ssap() access pdu->data[0] and pdu->data[1]
  unconditionally.  The subsequent computation:

    tlv_len = skb->len - LLCP_HEADER_SIZE;   /* LLCP_HEADER_SIZE = 2 */

  truncates to u16.  If skb->len < 2, the unsigned subtraction wraps at
  unsigned int width and the truncation to u16 yields up to 65534, causing
  the while loop to iterate far beyond the skb data.  No guard exists at
  the dispatch path to prevent this.

  Fix: add `if (skb->len < LLCP_HEADER_SIZE) return;` before any skb->data
  access, matching the pattern already used in nfc_llcp_recv_agf().

Issue 2 - missing per-iteration TLV header guard:

  The loop reads tlv[0] and tlv[1] with no prior check that two bytes
  remain.  When one byte remains, tlv[1] is one byte past the array end.

  Fix: `if (tlv_len - offset < 2) break;`

Issue 3 - peer-controlled `length` field advances tlv past skb end:

  `length` (tlv[1]) is advanced unconditionally into `offset` and `tlv`
  without verifying that `length` bytes of TLV value exist.  A malicious
  peer sets `length` large enough that `offset` remains below `tlv_len` on
  the next iteration while `tlv` points into adjacent kernel heap.

  Fix: `if (tlv_len - offset - 2 < length) break;`

Issue 4 - per-type minimum-length hazards:

  LLCP_TLV_SDREQ: `service_name_len = length - 1` is u8 arithmetic.  When
  length == 0 this wraps to 255, causing a 255-byte kernel memory scan via
  strncmp.  tlv[2] (tid) is also accessed unconditionally.
  Fix: require length >= 1 before the tid/service_name access.

  LLCP_TLV_SDRES: tlv[2] and tlv[3] are accessed without verifying
  length >= 2.  Unlike the GB/connection parsers, SDREQ/SDRES are not
  processed via llcp_tlv8/16, so the llcp_tlv_length[] table provides no
  protection here.
  Fix: require length >= 2 before the tlv[2]/tlv[3] accesses.

  In both cases a `break` from the inner switch falls through to the
  unconditional `offset += length + 2; tlv += length + 2` at the loop
  tail, correctly advancing past the malformed TLV.  The outer two guards
  break from the while loop entirely.

Reachability: SNL PDUs are processed during LLCP service discovery, before
any connection is established, from any NFC peer within ~4 cm with no
authentication or pairing.

Fixes: 19cfe5843e86 ("NFC: Initial SNL support")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/llcp_core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index db5bc6a87..16acf7c2b 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1284,6 +1284,11 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
 	size_t sdres_tlvs_len;
 	HLIST_HEAD(nl_sdres_list);
 
+	if (skb->len < LLCP_HEADER_SIZE) {
+		pr_err("Malformed SNL PDU\n");
+		return;
+	}
+
 	dsap = nfc_llcp_dsap(skb);
 	ssap = nfc_llcp_ssap(skb);
 
@@ -1300,11 +1305,17 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
 	sdres_tlvs_len = 0;
 
 	while (offset < tlv_len) {
+		if (tlv_len - offset < 2)
+			break;
 		type = tlv[0];
 		length = tlv[1];
+		if (tlv_len - offset - 2 < length)
+			break;
 
 		switch (type) {
 		case LLCP_TLV_SDREQ:
+			if (length < 1)
+				break;
 			tid = tlv[2];
 			service_name = (char *) &tlv[3];
 			service_name_len = length - 1;
@@ -1369,6 +1380,8 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
 			break;
 
 		case LLCP_TLV_SDRES:
+			if (length < 2)
+				break;
 			mutex_lock(&local->sdreq_lock);
 
 			pr_debug("LLCP_TLV_SDRES: searching tid %d\n", tlv[2]);
-- 
2.51.0