[PATCH] tools/mm/slabinfo.c: fix access to null terminator in string

[PATCH] tools/mm/slabinfo.c: fix access to null terminator in string

[Kaushlendra Kumar]

The current code incorrectly accesses buffer[strlen(buffer)],
which points to the null terminator ('\0') at the end of the
string. This is technically out-of-bounds access since valid
string content ends at index strlen(buffer)-1.

Fix by:
1. Storing strlen() result to avoid redundant calls
2. Adding bounds check (len > 0) to handle empty strings
3. Using buffer[len-1] to correctly access the last character
before the null terminator

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 tools/mm/slabinfo.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index 1433eff99feb..ac0cc6c1c87e 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -165,8 +165,10 @@ static unsigned long read_obj(const char *name)
 		if (!fgets(buffer, sizeof(buffer), f))
 			buffer[0] = 0;
 		fclose(f);
-		if (buffer[strlen(buffer)] == '\n')
-			buffer[strlen(buffer)] = 0;
+		size_t len = strlen(buffer);
+
+		if (len > 0 && buffer[len - 1] == '\n')
+			buffer[len - 1] = 0;
 	}
 	return strlen(buffer);
 }
-- 
2.34.1

Re: [PATCH] tools/mm/slabinfo.c: fix access to null terminator in string

[SeongJae Park]

Hello Kaushlendra,

On Fri, 29 Aug 2025 14:17:38 +0530 Kaushlendra Kumar <kaushlendra.kumar@intel.com> wrote:

> The current code incorrectly accesses buffer[strlen(buffer)],
> which points to the null terminator ('\0') at the end of the
> string. This is technically out-of-bounds access since valid
> string content ends at index strlen(buffer)-1.
> 
> Fix by:
> 1. Storing strlen() result to avoid redundant calls
> 2. Adding bounds check (len > 0) to handle empty strings
> 3. Using buffer[len-1] to correctly access the last character
> before the null terminator
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  tools/mm/slabinfo.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..ac0cc6c1c87e 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -165,8 +165,10 @@ static unsigned long read_obj(const char *name)
>  		if (!fgets(buffer, sizeof(buffer), f))
>  			buffer[0] = 0;
>  		fclose(f);
> -		if (buffer[strlen(buffer)] == '\n')
> -			buffer[strlen(buffer)] = 0;
> +		size_t len = strlen(buffer);

I'd prefer not mixing declarations with statements.

> +
> +		if (len > 0 && buffer[len - 1] == '\n')
> +			buffer[len - 1] = 0;
>  	}
>  	return strlen(buffer);
>  }
> -- 
> 2.34.1


Thanks,
SJ

Re: [PATCH] tools/mm/slabinfo.c: fix access to null terminator in string

[Kumar, Kaushlendra]

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]


> I'd prefer not mixing declarations with statements.

Thank you for the feedback! I've addressed this in v2 by moving the
variable declaration to function scope.

v2 patch sent with the requested change.

Best regards,
Kaushlendra
________________________________
From: SeongJae Park <sj@kernel.org>
Sent: Saturday, August 30, 2025 12:51 AM
To: Kumar, Kaushlendra <kaushlendra.kumar@intel.com>
Cc: SeongJae Park <sj@kernel.org>; akpm@linux-foundation.org <akpm@linux-foundation.org>; linux-mm@kvack.org <linux-mm@kvack.org>
Subject: Re: [PATCH] tools/mm/slabinfo.c: fix access to null terminator in string

Hello Kaushlendra,

On Fri, 29 Aug 2025 14:17:38 +0530 Kaushlendra Kumar <kaushlendra.kumar@intel.com> wrote:

> The current code incorrectly accesses buffer[strlen(buffer)],
> which points to the null terminator ('\0') at the end of the
> string. This is technically out-of-bounds access since valid
> string content ends at index strlen(buffer)-1.
>
> Fix by:
> 1. Storing strlen() result to avoid redundant calls
> 2. Adding bounds check (len > 0) to handle empty strings
> 3. Using buffer[len-1] to correctly access the last character
> before the null terminator
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  tools/mm/slabinfo.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..ac0cc6c1c87e 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -165,8 +165,10 @@ static unsigned long read_obj(const char *name)
>                if (!fgets(buffer, sizeof(buffer), f))
>                        buffer[0] = 0;
>                fclose(f);
> -             if (buffer[strlen(buffer)] == '\n')
> -                     buffer[strlen(buffer)] = 0;
> +             size_t len = strlen(buffer);

I'd prefer not mixing declarations with statements.

> +
> +             if (len > 0 && buffer[len - 1] == '\n')
> +                     buffer[len - 1] = 0;
>        }
>        return strlen(buffer);
>  }
> --
> 2.34.1


Thanks,
SJ

[-- Attachment #2: Type: text/html, Size: 5377 bytes --]