[PATCH] nfc: llcp: fix slab-out-of-bounds read in nfc_llcp_wks_sap()

[PATCH] nfc: llcp: fix slab-out-of-bounds read in nfc_llcp_wks_sap()

[Ömer Mete Kaya]

nfc_llcp_wks_sap() passes service_name to pr_debug() using the %s
format specifier. The service_name buffer is allocated via kmemdup()
in llcp_sock_bind() and is not null-terminated, causing
__dynamic_pr_debug() to read beyond the allocated region:

  KASAN: slab-out-of-bounds Read in __dynamic_pr_debug

Fix by using %.*s with service_name_len to limit the output to the
actual length of the string.

Reported-by: syzbot+1e3df0852e82c21ca418@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1e3df0852e82c21ca418
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
 net/nfc/llcp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index cac1b5487064..fda236e4d9fd 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -341,7 +341,7 @@ static int nfc_llcp_wks_sap(const char *service_name, size_t service_name_len)
 {
 	int sap, num_wks;
 
-	pr_debug("%s\n", service_name);
+	pr_debug("%.*s\n", (int)service_name_len, service_name);
 
 	if (service_name == NULL)
 		return -EINVAL;
-- 
2.55.0

[PATCH net v2] nfc: llcp: fix slab-out-of-bounds read in nfc_llcp_wks_sap()

[Ömer Mete Kaya]

nfc_llcp_wks_sap() passes service_name to pr_debug() using the %s
format specifier. The service_name buffer is allocated via kmemdup()
in llcp_sock_bind() and is not null-terminated, causing
__dynamic_pr_debug() to read beyond the allocated region:

  KASAN: slab-out-of-bounds Read in __dynamic_pr_debug

Fix by using %.*s with service_name_len to limit the output to the
actual length of the string.

Reported-by: syzbot+1e3df0852e82c21ca418@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1e3df0852e82c21ca418
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
v2: Changed prefix to [PATCH net] but still I could not find the Fixes: tag both in the gitblame and the lore.
 net/nfc/llcp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index cac1b5487064..fda236e4d9fd 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -341,7 +341,7 @@ static int nfc_llcp_wks_sap(const char *service_name, size_t service_name_len)
 {
 	int sap, num_wks;
 
-	pr_debug("%s\n", service_name);
+	pr_debug("%.*s\n", (int)service_name_len, service_name);
 
 	if (service_name == NULL)
 		return -EINVAL;
-- 
2.55.0

[PATCH net v3] nfc: llcp: fix slab-out-of-bounds reads when logging service names

[Ömer Mete Kaya]

nfc_llcp_wks_sap() and nfc_llcp_build_sdreq_tlv() pass non-null-
terminated strings to pr_debug() using the %s format specifier.
The buffers are allocated via kmemdup() or come from netlink
attributes and are not guaranteed to be null-terminated, causing
__dynamic_pr_debug() to read beyond the allocated region:

  KASAN: slab-out-of-bounds Read in __dynamic_pr_debug

Fix both call sites by using %.*s with the explicit length to limit
the output to the actual length of the string.

Reported-by: syzbot+1e3df0852e82c21ca418@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1e3df0852e82c21ca418
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
v3: Also fix identical issue in nfc_llcp_build_sdreq_tlv() as
    suggested by Sashiko review.
 net/nfc/llcp_commands.c | 2 +-
 net/nfc/llcp_core.c     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index ca89fe967d6a..1213946ce91f 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -135,7 +135,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdreq_tlv(u8 tid, const char *uri,
 {
 	struct nfc_llcp_sdp_tlv *sdreq;
 
-	pr_debug("uri: %s, len: %zu\n", uri, uri_len);
+	pr_debug("uri: %.*s, len: %zu\n", (int)uri_len, uri);
 
 	/* sdreq->tlv_len is u8, takes uri_len, + 3 for header, + 1 for NULL */
 	if (WARN_ON_ONCE(uri_len > U8_MAX - 4))
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index cac1b5487064..fda236e4d9fd 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -341,7 +341,7 @@ static int nfc_llcp_wks_sap(const char *service_name, size_t service_name_len)
 {
 	int sap, num_wks;
 
-	pr_debug("%s\n", service_name);
+	pr_debug("%.*s\n", (int)service_name_len, service_name);
 
 	if (service_name == NULL)
 		return -EINVAL;
-- 
2.55.0

Re: [PATCH net v3] nfc: llcp: fix slab-out-of-bounds reads when logging service names

[Simon Horman]

On Sun, Sep 06, 2026 at 03:38:08AM +0300, Ömer Mete Kaya wrote:
> nfc_llcp_wks_sap() and nfc_llcp_build_sdreq_tlv() pass non-null-
> terminated strings to pr_debug() using the %s format specifier.
> The buffers are allocated via kmemdup() or come from netlink
> attributes and are not guaranteed to be null-terminated, causing
> __dynamic_pr_debug() to read beyond the allocated region:
> 
>   KASAN: slab-out-of-bounds Read in __dynamic_pr_debug
> 
> Fix both call sites by using %.*s with the explicit length to limit
> the output to the actual length of the string.
> 

As a patch for net, this needs a Fixes tag here
(no blank line between it and other tags).
> Reported-by: syzbot+1e3df0852e82c21ca418@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1e3df0852e82c21ca418
> Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
> ---
> v3: Also fix identical issue in nfc_llcp_build_sdreq_tlv() as
>     suggested by Sashiko review.
>  net/nfc/llcp_commands.c | 2 +-
>  net/nfc/llcp_core.c     | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
> index ca89fe967d6a..1213946ce91f 100644
> --- a/net/nfc/llcp_commands.c
> +++ b/net/nfc/llcp_commands.c
> @@ -135,7 +135,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdreq_tlv(u8 tid, const char *uri,
>  {
>  	struct nfc_llcp_sdp_tlv *sdreq;
>  
> -	pr_debug("uri: %s, len: %zu\n", uri, uri_len);
> +	pr_debug("uri: %.*s, len: %zu\n", (int)uri_len, uri);

This does not compile because the trailing uri_len argument is now missing.

>  
>  	/* sdreq->tlv_len is u8, takes uri_len, + 3 for header, + 1 for NULL */
>  	if (WARN_ON_ONCE(uri_len > U8_MAX - 4))

And some notes on process that I'd appreciate you keeping in mind:

1. Please wait at least 24h before posting updated revisions of patches
   CCed to the Netdev ML unless you receive a wavier from one of the
   Networking maintainers on the ML.

2. Please send updated revisions of patches as new email threads,
   not as replies to earlier versions (or any other email).

You can read more about the Netdev development process here:
https://docs.kernel.org/process/maintainer-netdev.html

Thanks!

Re: [PATCH net v3] nfc: llcp: fix slab-out-of-bounds reads when logging service names

[Ömer Mete Kaya]

On 9/8/26 18:41, Simon Horman wrote:
> On Sun, Sep 06, 2026 at 03:38:08AM +0300, Ömer Mete Kaya wrote:
>> nfc_llcp_wks_sap() and nfc_llcp_build_sdreq_tlv() pass non-null-
>> terminated strings to pr_debug() using the %s format specifier.
>> The buffers are allocated via kmemdup() or come from netlink
>> attributes and are not guaranteed to be null-terminated, causing
>> __dynamic_pr_debug() to read beyond the allocated region:
>>
>>   KASAN: slab-out-of-bounds Read in __dynamic_pr_debug
>>
>> Fix both call sites by using %.*s with the explicit length to limit
>> the output to the actual length of the string.
>>
> 
> As a patch for net, this needs a Fixes tag here
> (no blank line between it and other tags).

Sorry, I know that but I wasnt sure whether to add it since the bug
has been there since the function was introduced. I will add it in v4.
>> @@ -135,7 +135,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdreq_tlv(u8 tid, const char *uri,
>>  {
>>  	struct nfc_llcp_sdp_tlv *sdreq;
>>  
>> -	pr_debug("uri: %s, len: %zu\n", uri, uri_len);
>> +	pr_debug("uri: %.*s, len: %zu\n", (int)uri_len, uri);
> 
> This does not compile because the trailing uri_len argument is now missing.

Sorry, I normally test-build before sending but this
one slipped through.

Re: [PATCH net v3] nfc: llcp: fix slab-out-of-bounds reads when logging service names

[Simon Horman]

On Tue, Sep 08, 2026 at 07:12:38PM +0300, Ömer Mete Kaya wrote:
> 
> 
> On 9/8/26 18:41, Simon Horman wrote:
> > On Sun, Sep 06, 2026 at 03:38:08AM +0300, Ömer Mete Kaya wrote:
> >> nfc_llcp_wks_sap() and nfc_llcp_build_sdreq_tlv() pass non-null-
> >> terminated strings to pr_debug() using the %s format specifier.
> >> The buffers are allocated via kmemdup() or come from netlink
> >> attributes and are not guaranteed to be null-terminated, causing
> >> __dynamic_pr_debug() to read beyond the allocated region:
> >>
> >>   KASAN: slab-out-of-bounds Read in __dynamic_pr_debug
> >>
> >> Fix both call sites by using %.*s with the explicit length to limit
> >> the output to the actual length of the string.
> >>
> > 
> > As a patch for net, this needs a Fixes tag here
> > (no blank line between it and other tags).
> 
> Sorry, I know that but I wasnt sure whether to add it since the bug
> has been there since the function was introduced. I will add it in v4.

Understood. FTR, yes, a Fixes tag is needed in such cases.
The idea is to guide backporting of the patch to all versions
that are effected.

> >> @@ -135,7 +135,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdreq_tlv(u8 tid, const char *uri,
> >>  {
> >>  	struct nfc_llcp_sdp_tlv *sdreq;
> >>  
> >> -	pr_debug("uri: %s, len: %zu\n", uri, uri_len);
> >> +	pr_debug("uri: %.*s, len: %zu\n", (int)uri_len, uri);
> > 
> > This does not compile because the trailing uri_len argument is now missing.
> 
> Sorry, I normally test-build before sending but this
> one slipped through.

Stuff happens :)

Re: [PATCH net v2] nfc: llcp: fix slab-out-of-bounds read in nfc_llcp_wks_sap()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 (1)

- [Medium] Missing `Fixes:` tag on a [PATCH net] bug fix.

Pre-existing issues:
- [Medium] Prefix-match logic error in nfc_llcp_wks_sap()
  (net/nfc/llcp_core.c:351-357): the loop does `strncmp(wks[sap],…
- [Low] Unfixed sibling of the same defect class:
  nfc_llcp_build_sdreq_tlv() (net/nfc/llcp_commands.c:138) still does…

--

nfc: llcp: fix slab-out-of-bounds read in nfc_llcp_wks_sap()

This changes the pr_debug() in nfc_llcp_wks_sap() from %s to %.*s bounded
by service_name_len, so the non-NUL-terminated kmemdup()'d service name
copied in llcp_sock_bind() is no longer over-read by the printk format
scan.

Closes: https://syzkaller.appspot.com/bug?extid=1e3df0852e82c21ca418

> v2: Changed prefix to [PATCH net] but still I could not find the Fixes: tag both in the gitblame and the lore.

[Severity: Medium]
Regarding "still I could not find the Fixes: tag both in the gitblame and
the lore" - does git blame on the removed statement point at the original
LLCP support commit?

Blaming the pr_debug("%s\n", service_name); line in nfc_llcp_wks_sap() at
the baseline attributes it to:

  d646960f7986 ("NFC: Initial LLCP support")

Only the enclosing function signature was touched later, by 3df40eb3a2ea.
The unterminated kmemdup() of the user-supplied service name in
llcp_sock_bind() / llcp_sock_connect() comes from the same initial LLCP
series, so would:

  Fixes: d646960f7986 ("NFC: Initial LLCP support")

be the right tag here?  Without it the affected range (everything since
v3.3) has to be worked out by hand for stable backports, and the net tree
process asks for the tag on fixes.

> diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
> index cac1b5487064d..fda236e4d9fdc 100644
> --- a/net/nfc/llcp_core.c
> +++ b/net/nfc/llcp_core.c
> @@ -341,7 +341,7 @@ static int nfc_llcp_wks_sap(const char *service_name, size_t service_name_len)
>  {
>  	int sap, num_wks;
>  
> -	pr_debug("%s\n", service_name);
> +	pr_debug("%.*s\n", (int)service_name_len, service_name);

[Severity: Low]
This is a pre-existing issue and not introduced by this patch, but is
there a second copy of the same pattern that could be converted in the
same change?  nfc_llcp_build_sdreq_tlv() in net/nfc/llcp_commands.c does:

  pr_debug("uri: %s, len: %zu\n", uri, uri_len);

The uri here is nla_data() of NFC_SDP_ATTR_URI, passed in from
nfc_genl_llc_sdreq() together with nla_len().  The policy entry is:

  [NFC_SDP_ATTR_URI] = { .type = NLA_STRING,
                         .len = U8_MAX - 4 },

NLA_STRING only trims an optional trailing NUL, it does not require one,
and the rest of nfc_llcp_build_sdreq_tlv() is written length-based
(if (uri[uri_len - 1] == 0) sdreq->tlv_len--; and memcpy(sdreq->uri, uri,
uri_len)), so no terminator is assumed there either.

Note this one does not appear to be able to produce the KASAN report the
changelog quotes: the payload sits in the netlink skb linear area and the
format scan stops on a zero byte inside the zeroed skb_shared_info of the
same allocation:

  net/core/skbuff.c:__alloc_skb() {
      ...
      memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
      ...
  }

What is left is the unbounded %s printing adjacent attribute bytes plus up
to three bytes of uninitialised SKB_DATA_ALIGN padding into dmesg, and it
needs CAP_NET_ADMIN (NFC_CMD_LLC_SDREQ carries GENL_ADMIN_PERM) plus an
enabled dyndbg callsite.  Would

  pr_debug("uri: %.*s, len: %zu\n", (int)uri_len, uri, uri_len);

be worth folding in for consistency?  The other %s users of a URI look
fine, since sdreq->uri points into a kzalloc(sdreq->tlv_len + 1).

>  
>  	if (service_name == NULL)
>  		return -EINVAL;
>  

The unquoted remainder of nfc_llcp_wks_sap() is the comparison loop:

  net/nfc/llcp_core.c:nfc_llcp_wks_sap() {
      ...
      for (sap = 0; sap < num_wks; sap++) {
              if (wks[sap] == NULL)
                      continue;

              if (strncmp(wks[sap], service_name, service_name_len) == 0)
                      return sap;
      }
      ...
  }

[Severity: Medium]
This is a pre-existing issue that the patch does not introduce, but since
the change lands two lines above it: should this comparison also require
service_name_len == strlen(wks[sap])?

As written, any strict prefix of a well-known name matches, for example
strncmp("urn:nfc:sn:ip", "urn:nfc:sn:i", 12) == 0 returns sap 2, and even a
single byte "u" matches.

nfc_llcp_wks_sap() is only reached from nfc_llcp_get_sdp_ssap(), which
passes the name and length verbatim from llcp_sock_bind() and
llcp_sock_connect().  Both just kmemdup() the user-supplied name under
min_t(..., NFC_LLCP_MAX_SERVICE_NAME) with no content validation, and
nfc_sock_create() does no capability check.

nfc_llcp_get_sdp_ssap() then treats the socket as the well-known service:

  net/nfc/llcp_core.c:nfc_llcp_get_sdp_ssap() {
      ...
      if (test_bit(ssap, &local->local_wks)) { ... return LLCP_SAP_MAX; }
      set_bit(ssap, &local->local_wks);
      return ssap;
      ...
  }

So a bind of "urn:nfc:sn:i" reserves the SAP belonging to
"urn:nfc:sn:ip", advertises it to the peer in the LLCP WKS general bytes,
and a later bind of the real name then fails with -EADDRINUSE.

The two peer-side sites in the same file do enforce exact length,
nfc_llcp_recv_snl() with:

  if (service_name_len == strlen("urn:nfc:sn:sdp") &&
      !strncmp(service_name, "urn:nfc:sn:sdp", service_name_len)) {

and nfc_llcp_sock_from_sn() skipping candidates whose service_name_len
differs before memcmp().  Is the missing length equality check here just an
oversight?  No out-of-bounds access is involved, strncmp reads at most
service_name_len bytes.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260905225211.596366-1-omermetekaya0%40gmail.com