[PATCH net 0/3] nfc: llcp: fix OOB reads in TLV parsers and PDU handlers

[PATCH net 0/3] nfc: llcp: fix OOB reads in TLV parsers and PDU handlers

[Lekë Hapçiu]

This series fixes three out-of-bounds read vulnerabilities in the NFC
LLCP layer, all reachable from RF without prior pairing or session
establishment.

Patch 1 adds missing TLV length bounds checks in nfc_llcp_parse_gb_tlv()
and nfc_llcp_parse_connection_tlv() — a crafted CONNECT or SNL PDU
containing a short TLV value field can read beyond the skb tail.

Patch 2 fixes nfc_llcp_recv_snl(), which accessed TLV fields and
performed arithmetic on an uncapped length byte before any bounds
check, enabling a 1-byte heap OOB read and a u8 wrap-around.

Patch 3 fixes nfc_llcp_recv_dm(), which read the DM reason byte at
skb->data[2] without verifying the frame is at least 3 bytes long.
A 2-byte DM PDU (header only) from a rogue peer triggers a 1-byte
OOB heap read.

All three bugs are independently triggered via RF (AV:A, AC:L, no
authentication required).

Lekë Hapçiu (3):
  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
  nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm()

 net/nfc/llcp_commands.c |  9 ++++++++-
 net/nfc/llcp_core.c     | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

--
2.34.1

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

[Lekë Hapçiu]

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: d646960f7986 ("NFC: Add LLCP sockets")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.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 2/3] nfc: llcp: fix TLV parsing OOB and length underflow in nfc_llcp_recv_snl

[Lekë Hapçiu]

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: 7a06f0ee2823 ("NFC: llcp: Service Name Lookup implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.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

[PATCH net 3/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
verifying that the frame is at least LLCP_HEADER_SIZE + 1 bytes long.
A rogue NFC peer can send a 2-byte DM PDU (header only, no reason
byte), triggering a 1-byte out-of-bounds read of kernel heap memory.
The same missing guard also leaves the nfc_llcp_dsap() and
nfc_llcp_ssap() macro accesses to data[0]/data[1] technically
unprotected against a 0- or 1-byte frame.

Add a single skb->len < LLCP_HEADER_SIZE + 1 check before any field
access, consistent with the guard added to nfc_llcp_recv_snl() by
commit ef8ddc69c ("nfc: llcp: fix bounds check in
nfc_llcp_recv_snl()").

The DM PDU is dispatched unconditionally by nfc_llcp_rx_skb() with no
prior length check, so this path is reachable from RF without any prior
pairing or session establishment.

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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1247,6 +1247,10 @@
 	struct nfc_llcp_sock *llcp_sock;
 	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);
--
2.34.1

Re: [PATCH net 0/3] nfc: llcp: fix OOB reads in TLV parsers and PDU handlers

[Paolo Abeni]

On 4/10/26 1:35 AM, Lekë Hapçiu wrote:
> This series fixes three out-of-bounds read vulnerabilities in the NFC
> LLCP layer, all reachable from RF without prior pairing or session
> establishment.
> 
> Patch 1 adds missing TLV length bounds checks in nfc_llcp_parse_gb_tlv()
> and nfc_llcp_parse_connection_tlv() — a crafted CONNECT or SNL PDU
> containing a short TLV value field can read beyond the skb tail.
> 
> Patch 2 fixes nfc_llcp_recv_snl(), which accessed TLV fields and
> performed arithmetic on an uncapped length byte before any bounds
> check, enabling a 1-byte heap OOB read and a u8 wrap-around.
> 
> Patch 3 fixes nfc_llcp_recv_dm(), which read the DM reason byte at
> skb->data[2] without verifying the frame is at least 3 bytes long.
> A 2-byte DM PDU (header only) from a rogue peer triggers a 1-byte
> OOB heap read.
> 
> All three bugs are independently triggered via RF (AV:A, AC:L, no
> authentication required).

This series looks like an older iteration of:

https://patchwork.kernel.org/user/todo/netdevbpf/?series=1079400

but it reached the ML 2h afterwards?!?

At very best you have some serious setup issue. Please have a look at
the repost policy and especially at the 24h grace period:

https://elixir.bootlin.com/linux/v7.0/source/Documentation/process/maintainer-netdev.rst

And, given the above problem, please do not share any more patches for
at least 48h.

/P