net/nfc/llcp_core.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-)
nfc_llcp_recv_snl() walked the SNL TLV list using a u16 offset/length
pair derived from skb->len, without bounding reads to the actual skb
data. Three problems followed:
- For a short frame (skb->len < LLCP_HEADER_SIZE), tlv_len underflowed.
- The per-TLV header (type, length) was read without checking that two
bytes remained.
- A declared TLV length could run past the end of the buffer, and an
SDREQ with length == 0 made "service_name_len = length - 1" underflow
(size_t), driving an out-of-bounds read in the following strncmp() /
nfc_llcp_sock_from_sn(). The SDRES case likewise read tlv[2]/tlv[3]
without a length check.
A nearby NFC device can reach this without authentication; LLCP link
activation happens automatically after NFC-DEP.
Walk the TLV list by pointer, bounded by skb_tail_pointer() over the
linear skb data, and validate each TLV declared length before use. Add
explicit length checks for SDREQ (>= 1) and SDRES (exactly 2).
Found by 0sec automated security-research tooling (https://0sec.ai).
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v3 (review cleanups, no functional change to the fix):
- Comment that only the linear part of the skb is parsed (David Laight).
- Use int for service_name_len and print the bounded service name
directly with %.*s; drop the min_t()/cast (David Laight).
- Require SDRES length to be exactly 2, not just >= 2 (David Laight).
v2: https://lore.kernel.org/netdev/20260603135935.62647-1-doruk@0sec.ai/
- Walk by pointer bounded on skb_tail_pointer(); drop the 16-bit
offset/tlv_len math and fix the short-frame underflow (David Laight).
- Add an SDRES length check alongside SDREQ length >= 1 (David Laight).
- Bound the SDREQ service-name pr_debug to the field length.
- Rebased onto linux-nfc for-next (David Heidelberg).
net/nfc/llcp_core.c | 29 +++++++++++++++++++++--------
1 file changed, 21 insertions(+), 8 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index dc65c719f..aed5fe1af 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -1286,10 +1286,9 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
{
struct nfc_llcp_sock *llcp_sock;
u8 dsap, ssap, type, length, tid, sap;
- const u8 *tlv;
- u16 tlv_len, offset;
+ const u8 *tlv, *tlv_end;
const char *service_name;
- size_t service_name_len;
+ int service_name_len;
struct nfc_llcp_sdp_tlv *sdp;
HLIST_HEAD(llc_sdres_list);
size_t sdres_tlvs_len;
@@ -1305,22 +1304,34 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
return;
}
+ /*
+ * Walk the SNL TLV list in the linear part of the skb only,
+ * bounded by skb_tail_pointer(). Each TLV needs a two-byte
+ * header (type, length) and its declared length must fit before
+ * the end; this also keeps the walk safe for very short frames.
+ */
tlv = &skb->data[LLCP_HEADER_SIZE];
- tlv_len = skb->len - LLCP_HEADER_SIZE;
- offset = 0;
+ tlv_end = skb_tail_pointer(skb);
sdres_tlvs_len = 0;
- while (offset < tlv_len) {
+ while (tlv + 2 < tlv_end) {
type = tlv[0];
length = tlv[1];
+ if (tlv + 2 + length > tlv_end)
+ break;
+
switch (type) {
case LLCP_TLV_SDREQ:
+ if (length < 1)
+ 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", service_name_len,
+ service_name);
if (service_name_len == strlen("urn:nfc:sn:sdp") &&
!strncmp(service_name, "urn:nfc:sn:sdp",
@@ -1380,6 +1391,9 @@ 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]);
@@ -1408,7 +1422,6 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local,
break;
}
- offset += length + 2;
tlv += length + 2;
}
--
2.53.0
On 04/06/2026 17:22, Doruk Tan Ozturk wrote: > nfc_llcp_recv_snl() walked the SNL TLV list using a u16 offset/length > pair derived from skb->len, without bounding reads to the actual skb > data. Three problems followed: > > - For a short frame (skb->len < LLCP_HEADER_SIZE), tlv_len underflowed. > - The per-TLV header (type, length) was read without checking that two > bytes remained. > - A declared TLV length could run past the end of the buffer, and an > SDREQ with length == 0 made "service_name_len = length - 1" underflow > (size_t), driving an out-of-bounds read in the following strncmp() / > nfc_llcp_sock_from_sn(). The SDRES case likewise read tlv[2]/tlv[3] > without a length check. > > A nearby NFC device can reach this without authentication; LLCP link > activation happens automatically after NFC-DEP. > > Walk the TLV list by pointer, bounded by skb_tail_pointer() over the > linear skb data, and validate each TLV declared length before use. Add > explicit length checks for SDREQ (>= 1) and SDRES (exactly 2). > > Found by 0sec automated security-research tooling (https://0sec.ai). This version looks much better! Since you fixing existing issue, adding Fixes tag and relevant commit which introduced the problem would be good too. Thank you for working on it. David > > Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai> > --- > v3 (review cleanups, no functional change to the fix): > - Comment that only the linear part of the skb is parsed (David Laight). > - Use int for service_name_len and print the bounded service name > directly with %.*s; drop the min_t()/cast (David Laight). > - Require SDRES length to be exactly 2, not just >= 2 (David Laight). > > v2: https://lore.kernel.org/netdev/20260603135935.62647-1-doruk@0sec.ai/ > - Walk by pointer bounded on skb_tail_pointer(); drop the 16-bit > offset/tlv_len math and fix the short-frame underflow (David Laight). > - Add an SDRES length check alongside SDREQ length >= 1 (David Laight). > - Bound the SDREQ service-name pr_debug to the field length. > - Rebased onto linux-nfc for-next (David Heidelberg). > > net/nfc/llcp_core.c | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > [...]
© 2016 - 2026 Red Hat, Inc.