[PATCH v2] tools/mm/slabinfo: fix access to null terminator in string boundary

[PATCH v2] tools/mm/slabinfo: fix access to null terminator in string boundary

[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. Declaring strlen() result variable at function scope
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

v2: Move variable declaration to function scope to avoid mixing
    declarations with statements (feedback from reviewer)

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

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index 1433eff99feb..d2e5b4e232b1 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -155,6 +155,8 @@ static void usage(void)
 
 static unsigned long read_obj(const char *name)
 {
+	size_t len;
+
 	FILE *f = fopen(name, "r");
 
 	if (!f) {
@@ -165,8 +167,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;
+		len = strlen(buffer);
+
+		if (len > 0 && buffer[len - 1] == '\n')
+			buffer[len - 1] = 0;
 	}
 	return strlen(buffer);
 }
-- 
2.34.1

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

[SeongJae Park]

On Sat, 30 Aug 2025 22:50:22 +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. Declaring strlen() result variable at function scope
> 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
> 
> v2: Move variable declaration to function scope to avoid mixing
>     declarations with statements (feedback from reviewer)

Thank you for making this change.  But, it is conventional to put the above
changelog on the comment section (after '---' line).

[1] https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format

> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  tools/mm/slabinfo.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..d2e5b4e232b1 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -155,6 +155,8 @@ static void usage(void)
>  
>  static unsigned long read_obj(const char *name)
>  {
> +	size_t len;
> +

I think the above blank line is not really needed.

>  	FILE *f = fopen(name, "r");
>  
>  	if (!f) {
> @@ -165,8 +167,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;
> +		len = strlen(buffer);
> +

The above balnk line seems not needed to me.

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

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

[Kumar, Kaushlendra]

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



________________________________
From: SeongJae Park <sj@kernel.org>
Sent: Monday, September 1, 2025 8:53 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 v2] tools/mm/slabinfo: fix access to null terminator in string boundary



> Thank you for making this change.  But, it is conventional to put the above
> changelog on the comment section (after '---' line).

> [1] https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format

Thank you for the feedback! I have incorporated your suggestions and submitted patch V4.
This version fixes an issue that remained unresolved in V3

> I think the above blank line is not really needed.
Addressed the comments



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

RE: [PATCH v2] tools/mm/slabinfo: fix access to null terminator in string boundary

[Kumar, Kaushlendra]

> Thank you for making this change.  But, it is conventional to put the above changelog on the comment section (after '---' line).
> 
> [1] https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
> 
> I think the above blank line is not really needed.

Thank you for the feedback! I have incorporated your suggestions and submitted patch V4.
This version fixes an issue that remained unresolved in V3.

BR,
Kaushlendra