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

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

Posted by Ömer Mete Kaya 2 weeks, 6 days ago

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

Posted by Simon Horman 2 weeks, 3 days ago

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

Posted by Ömer Mete Kaya 2 weeks, 3 days ago


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

Posted by Simon Horman 2 weeks, 3 days ago

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 :)