Re: [PATCH] perf sched: Fix crash in sched stats diff due to uninitialized perf_data structure

[James Clark <james.clark@linaro.org>]

On 22/04/2026 18:35, Athira Rajeev wrote:
> perf sched stats diff fails sometimes with below logs:
> 
>   # perf sched stats record ls
> 
>   # perf sched stats diff
>    *** invalid open64 call: O_CREAT or O_TMPFILE without mode ***: terminated
>    Aborted (core dumped)
> 
> Also in some runs, perf sched stats tests fails
> 
>   ./perf test "perf sched stats tests"
>   92: perf sched stats tests                                          : FAILED!
> 
> ==172375== Conditional jump or move depends on uninitialised value(s)
> ==172375==    at 0x10393BD0: open_file_read (data.c:264)
> ==172375==    by 0x10393F63: open_file (data.c:317)
> ==172375==    by 0x10394067: open_file_dup (data.c:334)
> ==172375==    by 0x103942DF: perf_data__open (data.c:379)
> ==172375==    by 0x102F7CAF: __perf_session__new (session.c:169)
> ==172375==    by 0x100B5E63: perf_session__new (session.h:116)
> ==172375==    by 0x100D5D37: perf_sched__schedstat_diff (builtin-sched.c:4568)
> ==172375==    by 0x100D7D6F: cmd_sched (builtin-sched.c:5061)
> ==172375==    by 0x1010231F: run_builtin (perf.c:348)
> ==172375==    by 0x1010262F: handle_internal_command (perf.c:398)
> ==172375==    by 0x1010286F: run_argv (perf.c:442)
> ==172375==    by 0x10102C67: main (perf.c:549)
> ==172375==
> ==172375== Conditional jump or move depends on uninitialised value(s)
> ==172375==    at 0x10393D20: open_file_read (data.c:282)
> ==172375==    by 0x10393F63: open_file (data.c:317)
> ==172375==    by 0x10394067: open_file_dup (data.c:334)
> ==172375==    by 0x103942DF: perf_data__open (data.c:379)
> ==172375==    by 0x102F7CAF: __perf_session__new (session.c:169)
> ==172375==    by 0x100B5E63: perf_session__new (session.h:116)
> ==172375==    by 0x100D5D37: perf_sched__schedstat_diff (builtin-sched.c:4568)
> ==172375==    by 0x100D7D6F: cmd_sched (builtin-sched.c:5061)
> ==172375==    by 0x1010231F: run_builtin (perf.c:348)
> ==172375==    by 0x1010262F: handle_internal_command (perf.c:398)
> ==172375==    by 0x1010286F: run_argv (perf.c:442)
> ==172375==    by 0x10102C67: main (perf.c:549)
> 
> The valgrind logs and error message from the logs points to
> below snippet:
> 
>       static int open_file_read(struct perf_data *data)
>       {
>              int flags = data->in_place_update ? O_RDWR : O_RDONLY;
>              struct stat st;
>              int fd;
> 
>              fd = open(data->file.path, flags);
> 
> The "struct perf_data" is defined here :
> 
>      static int perf_sched__schedstat_diff(struct perf_sched *sched,
>                                        int argc, const char **argv)
>      {
>          struct cpu_domain_map **cd_map0 = NULL, **cd_map1 = NULL;
>          struct list_head cpu_head_ses0, cpu_head_ses1;
>          struct perf_session *session[2];
>          struct perf_data data[2];
>          int ret = 0, err = 0;
>          static const char *defaults[] = {
>                  "perf.data.old",
>                  "perf.data",
>          };
> 
> Here due to uninitialized "struct perf_data", the arguments to "open"
> is getting wrongly passed sometimes depending on value in stack.
> When perf_data__open() function calls open_file_read(),the
> "in_place_update" could contain garbage value.
> 
> After initializing "struct perf_data" in perf_sched__schedstat_diff() function,
> perf sched stats tests runs without issues in multiple attempts and also
> "perf sched stats diff" ran individual without issues.
> 
> for i in {0..20}; do ./perf test "perf sched stats tests"; done
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
>   92: perf sched stats tests                                          : Ok
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.ibm.com>
> ---
>   tools/perf/builtin-sched.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 555247568e7a..5d2740099eed 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -4538,7 +4538,7 @@ static int perf_sched__schedstat_diff(struct perf_sched *sched,
>   	struct cpu_domain_map **cd_map0 = NULL, **cd_map1 = NULL;
>   	struct list_head cpu_head_ses0, cpu_head_ses1;
>   	struct perf_session *session[2];
> -	struct perf_data data[2];
> +	struct perf_data data[2] = { 0 };
>   	int ret = 0, err = 0;
>   	static const char *defaults[] = {
>   		"perf.data.old",

Reviewed-by: James Clark <james.clark@linaro.org>

I had a quick look for other instances of the same thing and didn't see any.