[PATCH net v4 0/5] nfc: fix multiple OOB reads in NCI and LLCP parsing paths

[PATCH net v4 0/5] nfc: fix multiple OOB reads in NCI and LLCP parsing paths

[Lekë Hapçiu]

This series fixes five out-of-bounds / underflow bugs in the kernel NFC
stack.  All are reachable from a remote NFC peer that the local stack
has already associated with; in the LLCP cases the peer only needs to
send a malformed frame.

  1/5  nci: u8 underflow in nci_store_general_bytes_nfc_dep() lets the
       attacker-controlled atr_res_len skip the GT-offset subtraction
       and cause an OOB read/write against general_bytes[].
  2/5  llcp: parse_gb_tlv() / parse_connection_tlv() trust the TLV
       length byte without checking remaining buffer, and the tlv16
       accessors read past the end when length < 2.
  3/5  llcp: nfc_llcp_recv_snl() has the same TLV-length trust bug, and
       its SDRES handler uses an unbounded "%.16s" pr_debug() that
       walks past service_name_len.
  4/5  llcp: nfc_llcp_recv_dm() reads skb->data[3] without checking
       skb->len, giving a 1-byte heap OOB read.
  5/5  llcp: nfc_llcp_connect_sn() walks the TLV array with no length
       validation; a crafted CONNECT frame drops it into OOB reads /
       an unbounded service-name pointer.

The series applies on top of net/main.

Lekë Hapçiu (5):
  nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
  nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv
  nfc: llcp: fix TLV parsing OOB in nfc_llcp_recv_snl
  nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm
  nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn

 net/nfc/llcp_commands.c | 24 ++++++++++++++++++++++--
 net/nfc/llcp_core.c     | 35 ++++++++++++++++++++++++++++++++---
 net/nfc/nci/ntf.c       |  6 ++++++
 3 files changed, 60 insertions(+), 5 deletions(-)

-- 
2.51.0

[PATCH net v4 3/5] nfc: llcp: fix TLV parsing OOB in nfc_llcp_recv_snl

[Lekë Hapçiu]

nfc_llcp_recv_snl() has five problems when handling a hostile peer:

 1. nfc_llcp_dsap()/nfc_llcp_ssap() dereference skb->data[0..1] without
    verifying skb->len; a 0- or 1-byte frame leads to an OOB read.
    Additionally tlv_len = skb->len - LLCP_HEADER_SIZE wraps when
    skb->len < 2, causing the following loop to run far past the
    buffer.

 2. The per-iteration loop guard `offset < tlv_len` only proves one
    byte is available, but the body reads tlv[0] and tlv[1].

 3. The peer-supplied `length` field is used to advance `tlv` without
    being checked against the remaining array space.

 4. The SDREQ handler reads tid (tlv[2]) and the first byte of
    service_name (tlv[3]) whenever length >= 1, so length == 1 reads
    one byte past the TLV.  A length of 2 is required to cover both
    accesses and to yield a non-empty service_name.

    The pr_debug("Looking for %.16s\n", service_name) additionally
    treats the buffer as a NUL-terminated string and can read up to
    15 bytes past service_name_len.  Use a length-bounded format
    ("%.*s") so the print cannot escape the validated TLV region.

 5. The SDRES handler reads tlv[2] (tid) and tlv[3] (sap) without
    checking length.  A length of 0 or 1 therefore triggers the same
    1- or 2-byte OOB read as SDREQ; require length >= 2 here as well.

Fix: reject frames smaller than LLCP_HEADER_SIZE up front; add TLV
header and TLV value guards at the top of each iteration; require
length >= 2 for SDREQ and SDRES; bound the pr_debug of service_name
with %.*s.

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

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index db5bc6a878dd..3284be517204 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,16 +1305,23 @@ 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 < 2)
+				break;
 			tid = tlv[2];
 			service_name = (char *) &tlv[3];
 			service_name_len = length - 1;
 
-			pr_debug("Looking for %.16s\n", service_name);
+			pr_debug("Looking for %.*s\n",
+				 (int)service_name_len, service_name);
 
 			if (service_name_len == strlen("urn:nfc:sn:sdp") &&
 			    !strncmp(service_name, "urn:nfc:sn:sdp",
@@ -1369,6 +1381,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 v4 4/5] 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
first verifying that skb->len is at least LLCP_HEADER_SIZE + 1.  A DM
PDU carrying only the 2-byte LLCP header from a rogue peer therefore
triggers a 1-byte OOB read.

Add the minimum-length guard at function entry, matching the pattern
used by nfc_llcp_recv_snl() and nfc_llcp_recv_agf().

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

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 3284be517204..ca0abfd329e5 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1239,6 +1239,11 @@ static void nfc_llcp_recv_dm(struct nfc_llcp_local *local,
 	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);
 	reason = skb->data[2];
-- 
2.51.0

[PATCH net v4 5/5] nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn

[Lekë Hapçiu]

nfc_llcp_connect_sn() walks the TLV array of an LLCP CONNECT PDU
looking for the Service Name TLV, but shares the same class of bugs
as nfc_llcp_recv_snl() / nfc_llcp_parse_gb_tlv():

 1. tlv_array_len = skb->len - LLCP_HEADER_SIZE wraps when skb->len
    is 0 or 1.  The subsequent loop then runs far past the buffer.

 2. The per-iteration guard `offset < tlv_array_len` only proves one
    byte is available, but the body reads both tlv[0] (type) and
    tlv[1] (length).

 3. The peer-supplied `length` field is used to advance `tlv` without
    being checked against the remaining array space, so a crafted
    length walks `tlv` past the buffer.  On the following iteration
    tlv[0]/tlv[1] are read from adjacent memory.

 4. When an LLCP_TLV_SN is found, the function returns &tlv[2] with
    *sn_len = length but without verifying that `length` bytes at
    tlv[2..] are still inside the TLV array.  The caller in
    nfc_llcp_recv_connect() then uses this (pointer, length) pair as
    a service name, so it may read past the PDU.

Fix: reject frames smaller than LLCP_HEADER_SIZE up front; add TLV
header and TLV value guards at the top of each iteration.  The value
guard also ensures that the (&tlv[2], length) pair returned on
LLCP_TLV_SN lies fully inside the TLV array.

Also use LLCP_HEADER_SIZE instead of the magic literal `2` to match
the style of neighbouring LLCP receive paths.

Reported-by: Simon Horman <horms@kernel.org>
Closes: https://lore.kernel.org/netdev/20260417160438.GH31784@horms.kernel.org/
Fixes: d646960f7986 ("NFC: Initial LLCP support")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <snowwlake@icloud.com>
---
 net/nfc/llcp_core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index ca0abfd329e5..df5567ca7fa8 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -849,12 +849,22 @@ static struct nfc_llcp_sock *nfc_llcp_sock_get_sn(struct nfc_llcp_local *local,
 static const u8 *nfc_llcp_connect_sn(const struct sk_buff *skb, size_t *sn_len)
 {
 	u8 type, length;
-	const u8 *tlv = &skb->data[2];
-	size_t tlv_array_len = skb->len - LLCP_HEADER_SIZE, offset = 0;
+	const u8 *tlv;
+	size_t tlv_array_len, offset = 0;
+
+	if (skb->len < LLCP_HEADER_SIZE)
+		return NULL;
+
+	tlv = &skb->data[LLCP_HEADER_SIZE];
+	tlv_array_len = skb->len - LLCP_HEADER_SIZE;
 
 	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);
 
-- 
2.51.0

Re: [PATCH net v4 0/5] nfc: fix multiple OOB reads in NCI and LLCP parsing paths

[Simon Horman]

On Fri, Apr 24, 2026 at 08:01:46PM +0200, Lekë Hapçiu wrote:
> This series fixes five out-of-bounds / underflow bugs in the kernel NFC
> stack.  All are reachable from a remote NFC peer that the local stack
> has already associated with; in the LLCP cases the peer only needs to
> send a malformed frame.
> 
>   1/5  nci: u8 underflow in nci_store_general_bytes_nfc_dep() lets the
>        attacker-controlled atr_res_len skip the GT-offset subtraction
>        and cause an OOB read/write against general_bytes[].
>   2/5  llcp: parse_gb_tlv() / parse_connection_tlv() trust the TLV
>        length byte without checking remaining buffer, and the tlv16
>        accessors read past the end when length < 2.
>   3/5  llcp: nfc_llcp_recv_snl() has the same TLV-length trust bug, and
>        its SDRES handler uses an unbounded "%.16s" pr_debug() that
>        walks past service_name_len.
>   4/5  llcp: nfc_llcp_recv_dm() reads skb->data[3] without checking
>        skb->len, giving a 1-byte heap OOB read.
>   5/5  llcp: nfc_llcp_connect_sn() walks the TLV array with no length
>        validation; a crafted CONNECT frame drops it into OOB reads /
>        an unbounded service-name pointer.
> 
> The series applies on top of net/main.
> 
> Lekë Hapçiu (5):
>   nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
>   nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv
>   nfc: llcp: fix TLV parsing OOB in nfc_llcp_recv_snl
>   nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm
>   nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn

Hi,

My only feedback on v4 of this patchset is that somehow the
threading is broken: each of patch 1/5 - 5/5 should be a reply
to the cover letter - 0/5 - but that does not seem to be the case.
And some tooling, notably Sashiko, seems to rely on the
entire patchset being contained in a single email thread.

Re: [PATCH net v4 0/5] nfc: fix multiple OOB reads in NCI and LLCP parsing paths

[Paolo Abeni]

On 4/28/26 2:55 PM, Simon Horman wrote:
> On Fri, Apr 24, 2026 at 08:01:46PM +0200, Lekë Hapçiu wrote:
>> This series fixes five out-of-bounds / underflow bugs in the kernel NFC
>> stack.  All are reachable from a remote NFC peer that the local stack
>> has already associated with; in the LLCP cases the peer only needs to
>> send a malformed frame.
>>
>>   1/5  nci: u8 underflow in nci_store_general_bytes_nfc_dep() lets the
>>        attacker-controlled atr_res_len skip the GT-offset subtraction
>>        and cause an OOB read/write against general_bytes[].
>>   2/5  llcp: parse_gb_tlv() / parse_connection_tlv() trust the TLV
>>        length byte without checking remaining buffer, and the tlv16
>>        accessors read past the end when length < 2.
>>   3/5  llcp: nfc_llcp_recv_snl() has the same TLV-length trust bug, and
>>        its SDRES handler uses an unbounded "%.16s" pr_debug() that
>>        walks past service_name_len.
>>   4/5  llcp: nfc_llcp_recv_dm() reads skb->data[3] without checking
>>        skb->len, giving a 1-byte heap OOB read.
>>   5/5  llcp: nfc_llcp_connect_sn() walks the TLV array with no length
>>        validation; a crafted CONNECT frame drops it into OOB reads /
>>        an unbounded service-name pointer.
>>
>> The series applies on top of net/main.
>>
>> Lekë Hapçiu (5):
>>   nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
>>   nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv
>>   nfc: llcp: fix TLV parsing OOB in nfc_llcp_recv_snl
>>   nfc: llcp: fix OOB read of DM reason byte in nfc_llcp_recv_dm
>>   nfc: llcp: fix TLV parsing OOB in nfc_llcp_connect_sn
> 
> Hi,
> 
> My only feedback on v4 of this patchset is that somehow the
> threading is broken: each of patch 1/5 - 5/5 should be a reply
> to the cover letter - 0/5 - but that does not seem to be the case.
> And some tooling, notably Sashiko, seems to rely on the
> entire patchset being contained in a single email thread.

Given the above, I suggest re-posting.

Also note that we are moving NFC to a specific subtree, see:

https://lore.kernel.org/netdev/938496c6-84c1-4d53-bb56-73bbd7b2bdd7@ixit.cz/

please wait a bit for resubmission, possibly David will be already ready
to catch them.

Thanks,

Paolo