Re: [PATCH] tools/sched_ext: scx_flatcg: Fix uninitialized stats on allocation failure

Hi Liang,

On Mon, Jul 13, 2026 at 03:18:08PM +0800, [email protected] wrote:
> From: Liang Luo <[email protected]>
> 
> In fcg_read_stats(), the memset() that zeroes the output @stats array
> sits after the calloc() failure check. When calloc() fails, the
> function returns without writing @stats.
> 
> The caller in main() declares acc_stats uninitialized, passes it as
> the @stats argument, and then reads it unconditionally:
> 
>     __u64 acc_stats[FCG_NR_STATS];
>     fcg_read_stats(skel, acc_stats);
>     stats[i] = acc_stats[i] - last_stats[i];   // reads garbage
> 
> Because fcg_read_stats() returns void, the caller cannot detect the
> failure. Reading the uninitialized array is undefined behavior, and
> the garbage is further copied into last_stats via memcpy(), corrupting
> the baseline used by the next interval.
> 
> This regression was introduced by commit cabd76bbc036 ("tools/sched_ext:
> scx_flatcg: fix potential stack overflow from VLA in fcg_read_stats"),
> which replaced the VLA with calloc() and inserted the failure check
> before the existing memset().
> 
> Move the memset() above the calloc() failure check so @stats is always
> zeroed regardless of allocation outcome.
> 
> Fixes: cabd76bbc036 ("tools/sched_ext: scx_flatcg: fix potential stack overflow from VLA in fcg_read_stats")
> Signed-off-by: Liang Luo <[email protected]>

Makes sense.

Reviewed-by: Andrea Righi <[email protected]>

Thanks,
-Andrea

> ---
>  tools/sched_ext/scx_flatcg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/sched_ext/scx_flatcg.c b/tools/sched_ext/scx_flatcg.c
> index de2bef86d64d..7799782b76d1 100644
> --- a/tools/sched_ext/scx_flatcg.c
> +++ b/tools/sched_ext/scx_flatcg.c
> @@ -105,12 +105,12 @@ static void fcg_read_stats(struct scx_flatcg *skel, __u64 *stats)
>  	__u64 *cnts;
>  	__u32 idx;
>  
> +	memset(stats, 0, sizeof(stats[0]) * FCG_NR_STATS);
> +
>  	cnts = calloc(skel->rodata->nr_cpus, sizeof(__u64));
>  	if (!cnts)
>  		return;
>  
> -	memset(stats, 0, sizeof(stats[0]) * FCG_NR_STATS);
> -
>  	for (idx = 0; idx < FCG_NR_STATS; idx++) {
>  		int ret, cpu;
>  
> -- 
> 2.43.0
>