* [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations
@ 2025-08-29 9:59 Kaushlendra Kumar
2025-08-29 11:51 ` Harry Yoo
2025-08-29 18:52 ` SeongJae Park
0 siblings, 2 replies; 5+ messages in thread
From: Kaushlendra Kumar @ 2025-08-29 9:59 UTC (permalink / raw)
To: akpm; +Cc: linux-mm, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations
2025-08-29 9:59 [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations Kaushlendra Kumar
@ 2025-08-29 11:51 ` Harry Yoo
2025-08-29 13:12 ` Kumar, Kaushlendra
2025-08-29 15:30 ` Kumar, Kaushlendra
2025-08-29 18:52 ` SeongJae Park
1 sibling, 2 replies; 5+ messages in thread
From: Harry Yoo @ 2025-08-29 11:51 UTC (permalink / raw)
To: Kaushlendra Kumar; +Cc: akpm, linux-mm, vbabka, cl, rientjes, roman.gushchin
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations
2025-08-29 11:51 ` Harry Yoo
@ 2025-08-29 13:12 ` Kumar, Kaushlendra
2025-08-29 15:30 ` Kumar, Kaushlendra
1 sibling, 0 replies; 5+ messages in thread
From: Kumar, Kaushlendra @ 2025-08-29 13:12 UTC (permalink / raw)
To: Harry Yoo
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, vbabka@suse.cz,
cl@linux.com, rientjes@google.com, roman.gushchin@linux.dev
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations
2025-08-29 11:51 ` Harry Yoo
2025-08-29 13:12 ` Kumar, Kaushlendra
@ 2025-08-29 15:30 ` Kumar, Kaushlendra
1 sibling, 0 replies; 5+ messages in thread
From: Kumar, Kaushlendra @ 2025-08-29 15:30 UTC (permalink / raw)
To: Harry Yoo
Cc: akpm@linux-foundation.org, linux-mm@kvack.org, vbabka@suse.cz,
cl@linux.com, rientjes@google.com, roman.gushchin@linux.dev
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations
2025-08-29 9:59 [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations Kaushlendra Kumar
2025-08-29 11:51 ` Harry Yoo
@ 2025-08-29 18:52 ` SeongJae Park
1 sibling, 0 replies; 5+ messages in thread
From: SeongJae Park @ 2025-08-29 18:52 UTC (permalink / raw)
To: Kaushlendra Kumar; +Cc: SeongJae Park, akpm, linux-mm
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
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-08-29 18:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 9:59 [PATCH] tools/mm/slabinfo: fix buffer overflows in fread operations Kaushlendra Kumar
2025-08-29 11:51 ` Harry Yoo
2025-08-29 13:12 ` Kumar, Kaushlendra
2025-08-29 15:30 ` Kumar, Kaushlendra
2025-08-29 18:52 ` SeongJae Park
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).