[PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[Kaushlendra Kumar]

The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
up to sizeof(buffer) bytes, but then unconditionally write a null
terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
beyond the allocated buffer boundaries.

Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
ensuring space is always reserved for null termination. This prevents
buffer overflows while maintaining proper string handling.

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

diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
index 1433eff99feb..1a7f2874c625 100644
--- a/tools/mm/slabinfo.c
+++ b/tools/mm/slabinfo.c
@@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
 		buffer[0] = 0;
 		l = 0;
 	} else {
-		l = fread(buffer, 1, sizeof(buffer), f);
+		l = fread(buffer, 1, sizeof(buffer) - 1, f);
 		buffer[l] = 0;
 		fclose(f);
 	}
@@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
 		buffer[0] = 0;
 		l = 0;
 	} else {
-		l = fread(buffer, 1, sizeof(buffer), f);
+		l = fread(buffer, 1, sizeof(buffer) - 1, f);
 		buffer[l] = 0;
 		fclose(f);
 	}
-- 
2.34.1

Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[Harry Yoo]

On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
> 
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

A side question, did you observe this while using the tool?
Perhaps that means we need to make the buffer bigger.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>  		buffer[0] = 0;
>  		l = 0;
>  	} else {
> -		l = fread(buffer, 1, sizeof(buffer), f);
> +		l = fread(buffer, 1, sizeof(buffer) - 1, f);
>  		buffer[l] = 0;
>  		fclose(f);
>  	}
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>  		buffer[0] = 0;
>  		l = 0;
>  	} else {
> -		l = fread(buffer, 1, sizeof(buffer), f);
> +		l = fread(buffer, 1, sizeof(buffer) - 1, f);
>  		buffer[l] = 0;
>  		fclose(f);
>  	}
> -- 
> 2.34.1

-- 
Cheers,
Harry / Hyeonggon

Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[Kumar, Kaushlendra]

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

On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can
> read up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this
> writes beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
>
> A side question, did you observe this while using the tool?
> Perhaps that means we need to make the buffer bigger.

Thanks for the review!

I discovered this issue while testing some local modifications to the code.
For now, let's keep the current buffer size and address the overflow with this fix.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c index
> 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> --
> 2.34.1


________________________________
From: Harry Yoo <harry.yoo@oracle.com>
Sent: Friday, August 29, 2025 5:21 PM
To: Kumar, Kaushlendra <kaushlendra.kumar@intel.com>
Cc: akpm@linux-foundation.org <akpm@linux-foundation.org>; linux-mm@kvack.org <linux-mm@kvack.org>; vbabka@suse.cz <vbabka@suse.cz>; cl@linux.com <cl@linux.com>; rientjes@google.com <rientjes@google.com>; roman.gushchin@linux.dev <roman.gushchin@linux.dev>
Subject: Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

A side question, did you observe this while using the tool?
Perhaps that means we need to make the buffer bigger.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> --
> 2.34.1

--
Cheers,
Harry / Hyeonggon

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

Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[Kumar, Kaushlendra]

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

Harry Yoo
Aug. 29, 2025, 11:51 a.m. UTC | #1
On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
>
> A side question, did you observe this while using the tool?
> Perhaps that means we need to make the buffer bigger.

Found during Code Review

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> --
> 2.34.1

Kumar, Kaushlendra
Aug. 29, 2025, 1:12 p.m. UTC | #2
On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can
> read up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this
> writes beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

> Reviewed-by: Harry Yoo <harry.yoo@oracle.com>
>
> A side question, did you observe this while using the tool?
> Perhaps that means we need to make the buffer bigger.

Thanks for the review!

I discovered this issue while testing some local modifications to the code.
For now, let's keep the current buffer size and address the overflow with this fix.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c index
> 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>           buffer[0] = 0;
>           l = 0;
>     } else {
> -         l = fread(buffer, 1, sizeof(buffer), f);
> +         l = fread(buffer, 1, sizeof(buffer) - 1, f);
>           buffer[l] = 0;
>           fclose(f);
>     }
> --
> 2.34.1


________________________________
From: Harry Yoo <harry.yoo@oracle.com>
Sent: Friday, August 29, 2025 5:21 PM
To: Kumar, Kaushlendra <kaushlendra.kumar@intel.com>
Cc: akpm@linux-foundation.org <akpm@linux-foundation.org>; linux-mm@kvack.org <linux-mm@kvack.org>; vbabka@suse.cz <vbabka@suse.cz>; cl@linux.com <cl@linux.com>; rientjes@google.com <rientjes@google.com>; roman.gushchin@linux.dev <roman.gushchin@linux.dev>
Subject: Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

A side question, did you observe this while using the tool?
Perhaps that means we need to make the buffer bigger.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> --
> 2.34.1

--
Cheers,
Harry / Hyeonggon


________________________________
From: Harry Yoo <harry.yoo@oracle.com>
Sent: Friday, August 29, 2025 5:21 PM
To: Kumar, Kaushlendra <kaushlendra.kumar@intel.com>
Cc: akpm@linux-foundation.org <akpm@linux-foundation.org>; linux-mm@kvack.org <linux-mm@kvack.org>; vbabka@suse.cz <vbabka@suse.cz>; cl@linux.com <cl@linux.com>; rientjes@google.com <rientjes@google.com>; roman.gushchin@linux.dev <roman.gushchin@linux.dev>
Subject: Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

On Fri, Aug 29, 2025 at 03:29:47PM +0530, Kaushlendra Kumar wrote:
> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
>
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---

Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

A side question, did you observe this while using the tool?
Perhaps that means we need to make the buffer bigger.

>  tools/mm/slabinfo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mm/slabinfo.c b/tools/mm/slabinfo.c
> index 1433eff99feb..1a7f2874c625 100644
> --- a/tools/mm/slabinfo.c
> +++ b/tools/mm/slabinfo.c
> @@ -228,7 +228,7 @@ static unsigned long read_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> @@ -247,7 +247,7 @@ static unsigned long read_debug_slab_obj(struct slabinfo *s, const char *name)
>                buffer[0] = 0;
>                l = 0;
>        } else {
> -             l = fread(buffer, 1, sizeof(buffer), f);
> +             l = fread(buffer, 1, sizeof(buffer) - 1, f);
>                buffer[l] = 0;
>                fclose(f);
>        }
> --
> 2.34.1

--
Cheers,
Harry / Hyeonggon

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

Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[SeongJae Park]

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

> The fread() calls in read_slab_obj() and read_debug_slab_obj() can read
> up to sizeof(buffer) bytes, but then unconditionally write a null
> terminator at buffer[l]. If fread() returns sizeof(buffer), this writes
> beyond the allocated buffer boundaries.
> 
> Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions,
> ensuring space is always reserved for null termination. This prevents
> buffer overflows while maintaining proper string handling.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>

Acked-by: SeongJae Park <sj@kernel.org>


Thanks,
SJ

[...]

RE: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[Kumar, Kaushlendra]

On Fri, 29 Aug 2025, SeongJae Park wrote:
> On Fri, 29 Aug 2025 15:29:47 +0530 Kaushlendra Kumar <kaushlendra.kumar@intel.com> wrote:
> 
> > The fread() calls in read_slab_obj() and read_debug_slab_obj() can 
> > read up to sizeof(buffer) bytes, but then unconditionally write a null 
> > terminator at buffer[l]. If fread() returns sizeof(buffer), this 
> > writes beyond the allocated buffer boundaries.
> > 
> > Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions, 
> > ensuring space is always reserved for null termination. This prevents 
> > buffer overflows while maintaining proper string handling.
> > 
> > Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> 
> Acked-by: SeongJae Park <sj@kernel.org>
> 
> 
> Thanks,
> SJ

Hi SeongJae,

Thank you for the Acked-by! 

Since this patch has received your acknowledgment, are we going to merge this 
patch? Should I expect it to be picked up for the next kernel release, or 
are there any additional steps needed from my side?

I appreciate your review and guidance on the merge process.

Best regards,
Kaushlendra

RE: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations

[SeongJae Park]

On Mon, 22 Sep 2025 02:40:11 +0000 "Kumar, Kaushlendra" <kaushlendra.kumar@intel.com> wrote:

> On Fri, 29 Aug 2025, SeongJae Park wrote:
> > On Fri, 29 Aug 2025 15:29:47 +0530 Kaushlendra Kumar <kaushlendra.kumar@intel.com> wrote:
> > 
> > > The fread() calls in read_slab_obj() and read_debug_slab_obj() can 
> > > read up to sizeof(buffer) bytes, but then unconditionally write a null 
> > > terminator at buffer[l]. If fread() returns sizeof(buffer), this 
> > > writes beyond the allocated buffer boundaries.
> > > 
> > > Fix by limiting reads to sizeof(buffer) - 1 bytes in both functions, 
> > > ensuring space is always reserved for null termination. This prevents 
> > > buffer overflows while maintaining proper string handling.
> > > 
> > > Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> > 
> > Acked-by: SeongJae Park <sj@kernel.org>
> > 
> > 
> > Thanks,
> > SJ
> 
> Hi SeongJae,
> 
> Thank you for the Acked-by! 
> 
> Since this patch has received your acknowledgment, are we going to merge this 
> patch? Should I expect it to be picked up for the next kernel release, or 
> are there any additional steps needed from my side?

I'm not a maintainer or a reviewr of slabinfo, so my Acked-by: doesn't mean
many things.  I think the next steps and decisions are up to Andrew.

And I think Andrew should be busy for the preparation of the next merge window.
I'd suggest pinging Andrew again, after the next MM pull requests for 6.18-rc1
are done.


Thanks,
SJ

[...]