[PATCH net] nfc: llcp: fix tlv offset wrap and missing bounds checks

[PATCH net] nfc: llcp: fix tlv offset wrap and missing bounds checks

[Oleh Konko]

nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() iterate a
u16 tlv_array_len with a u8 offset. once cumulative TLV consumption
crosses 255 bytes, offset wraps and the loop may continue past the
declared TLV array bounds.

both parsers also read tlv[1] before checking that a full 2-byte TLV
header remains, and they advance by length + 2 without validating that
the declared payload still fits in the remaining array.

fix this by widening offset to u16 and by rejecting incomplete headers
or truncated TLVs before dereferencing or advancing the cursor.

Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Reported-by: Oleh Konko <security@1seal.org>
Signed-off-by: Oleh Konko <security@1seal.org>
---
 net/nfc/llcp_commands.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 291f26fac..157afd62f 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);
 
@@ -201,6 +202,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 		return -ENODEV;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
+
 		type = tlv[0];
 		length = tlv[1];
 
@@ -227,6 +231,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 			break;
 		}
 
+		if (tlv_array_len - offset < (u16)length + 2)
+			return -EINVAL;
+
 		offset += length + 2;
 		tlv += length + 2;
 	}
@@ -243,7 +250,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);
 
@@ -251,6 +259,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 		return -ENOTCONN;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
+
 		type = tlv[0];
 		length = tlv[1];
 
@@ -270,6 +281,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 			break;
 		}
 
+		if (tlv_array_len - offset < (u16)length + 2)
+			return -EINVAL;
+
 		offset += length + 2;
 		tlv += length + 2;
 	}
-- 
2.50.0

Re: [PATCH net] nfc: llcp: fix tlv offset wrap and missing bounds checks

[Simon Horman]

On Tue, Mar 24, 2026 at 09:25:41PM +0000, Oleh Konko wrote:
> nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() iterate a
> u16 tlv_array_len with a u8 offset. once cumulative TLV consumption
> crosses 255 bytes, offset wraps and the loop may continue past the
> declared TLV array bounds.
> 
> both parsers also read tlv[1] before checking that a full 2-byte TLV
> header remains, and they advance by length + 2 without validating that
> the declared payload still fits in the remaining array.
> 
> fix this by widening offset to u16 and by rejecting incomplete headers
> or truncated TLVs before dereferencing or advancing the cursor.

This patch seems to fix two problems.  It's usually best to fix one problem
per patch, creating a patch-set with two or more patches and a cover letter
if necessary.

> 
> Fixes: d646960f7986 ("NFC: Initial LLCP support")
> Cc: stable@vger.kernel.org
> Reported-by: Oleh Konko <security@1seal.org>
> Signed-off-by: Oleh Konko <security@1seal.org>

This patch seems to have been sent twice in about half an hour.
Please don't do that.

Rather, please observe a pause of 24h between sending updated
revisions of a patch.

Link: https://docs.kernel.org/process/maintainer-netdev.html

> ---
>  net/nfc/llcp_commands.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
> index 291f26fac..157afd62f 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;

It seems to me that the value passed to this function by the caller,
nfc_llcp_set_remote_gb(), is limited to a maximum value of 255 - 3 by
virtue of being a u8 with 3 subtracted. So I'm not sure that offset
can overflow here.

For consistency with other parts of this call-chain
it might be worth changing the type of tlv_array_len to u8.
But that would be a clean-up for net-next rather than a fix for net.

>  
>  	pr_debug("TLV array length %d\n", tlv_array_len);
>  
> @@ -201,6 +202,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
>  		return -ENODEV;
>  
>  	while (offset < tlv_array_len) {
> +		if (tlv_array_len - offset < 2)
> +			return -EINVAL;
> +
>  		type = tlv[0];
>  		length = tlv[1];
>  
> @@ -227,6 +231,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
>  			break;
>  		}
>  
> +		if (tlv_array_len - offset < (u16)length + 2)
> +			return -EINVAL;
> +
>  		offset += length + 2;
>  		tlv += length + 2;
>  	}
> @@ -243,7 +250,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;

In this case the callers pass skb->ken as tlv_array_len.
As skb->len is an unsigned int in theory it might overflow
the u16 used for tlv_array_len. But perhaps in practice that
doesn't occur?

In any case, if you are going to update the type of offset, then
I would just change the type of type, length and offset to unsigned int.
I don't see a value in using narrower types here.

>  
>  	pr_debug("TLV array length %d\n", tlv_array_len);
>  
> @@ -251,6 +259,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
>  		return -ENOTCONN;
>  
>  	while (offset < tlv_array_len) {
> +		if (tlv_array_len - offset < 2)
> +			return -EINVAL;
> +
>  		type = tlv[0];
>  		length = tlv[1];
>  
> @@ -270,6 +281,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
>  			break;
>  		}
>  
> +		if (tlv_array_len - offset < (u16)length + 2)
> +			return -EINVAL;
> +
>  		offset += length + 2;
>  		tlv += length + 2;
>  	}
> -- 
> 2.50.0
> 

[PATCH net] nfc: llcp: fix tlv offset wrap and missing bounds checks

[Oleh Konko]

nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() iterate a
u16 tlv_array_len with a u8 offset. once cumulative TLV consumption
crosses 255 bytes, offset wraps and the loop may continue past the
declared TLV array bounds.

both parsers also read tlv[1] before checking that a full 2-byte TLV
header remains, and they advance by length + 2 without validating that
the declared payload still fits in the remaining array.

fix this by widening offset to u16 and by rejecting incomplete headers
or truncated TLVs before dereferencing or advancing the cursor.

Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Reported-by: Oleh Konko <security@1seal.org>
Signed-off-by: Oleh Konko <security@1seal.org>
---
 net/nfc/llcp_commands.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 291f26fac..157afd62f 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);
 
@@ -201,6 +202,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 		return -ENODEV;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
+
 		type = tlv[0];
 		length = tlv[1];
 
@@ -227,6 +231,9 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 			break;
 		}
 
+		if (tlv_array_len - offset < (u16)length + 2)
+			return -EINVAL;
+
 		offset += length + 2;
 		tlv += length + 2;
 	}
@@ -243,7 +250,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);
 
@@ -251,6 +259,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 		return -ENOTCONN;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
+
 		type = tlv[0];
 		length = tlv[1];
 
@@ -270,6 +281,9 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 			break;
 		}
 
+		if (tlv_array_len - offset < (u16)length + 2)
+			return -EINVAL;
+
 		offset += length + 2;
 		tlv += length + 2;
 	}
-- 
2.50.0