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

Lekë Hapçiu
2026-04-09T19:00:49Z

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