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

[Harry Yoo <harry.yoo@oracle.com>]

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