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

["Lekë Hapçiu" <snowwlake@icloud.com>]

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