Re: [PATCH] perf synthetic-events: Check schedstat domain allocation failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free and double-free vulnerability in perf_event__synthesize_schedstat() when encountering unrecognized lines.
--

commit 6cad01b2d5eb6ff97735af9d1d5fd8e900c1d5a0
Author: Hui Su <sh_def@163.com>

perf synthetic-events: Check schedstat domain allocation failure

__synthesize_schedstat_domain() uses event after zalloc() without
checking for allocation failure, unlike __synthesize_schedstat_cpu().
perf_event__synthesize_schedstat() already treats a NULL event as an
error and cleans up, so return NULL when the allocation fails.

> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 0c150193cca83..24559691b88ee 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -2776,6 +2776,8 @@ static union perf_event *__synthesize_schedstat_domain(struct io *io, __u16 vers
>  	size = sizeof(*ds);
>  	size = PERF_ALIGN(size, sizeof(u64));
>  	event = zalloc(size);
> +	if (!event)
> +		return NULL;
>  
>  	ds = &event->schedstat_domain;
>  	ds->header.type = PERF_RECORD_SCHEDSTAT_DOMAIN;

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
in the caller perf_event__synthesize_schedstat().

If the loop in perf_event__synthesize_schedstat() encounters a line starting
with a character other than 'c' or 'd' (such as an unhandled trailing
newline or a new record type from a newer kernel):

tools/perf/util/synthetic-events.c:perf_event__synthesize_schedstat() {
...
        for (ch = io__get_char(&io); !io.eof; ch = io__get_char(&io)) {
                struct perf_cpu this_cpu;

                if (ch == 'c') {
                        event = __synthesize_schedstat_cpu(&io, version,
                                                           &cpu, timestamp);
                } else if (ch == 'd') {
                        event = __synthesize_schedstat_domain(&io, version,
                                                              cpu, timestamp);
                }
                if (!event)
                        goto out_free_line;
...
}

Does this check incorrectly assume the event pointer is NULL if an unrecognized
line is parsed?

Because the event pointer is freed at the end of the previous iteration but not
reset to NULL, it retains a dangling pointer. The check if (!event) will pass,
leading the code to actively pass the stale pointer to the process() callback:

tools/perf/util/synthetic-events.c:perf_event__synthesize_schedstat() {
...
                if (process(tool, event, NULL, NULL) < 0) {
                        free(event);
                        goto out_free_line;
                }

                free(event);
        }
...
}

Could this trigger a use-after-free and a subsequent double-free?

Would it be safer to initialize the event pointer to NULL at the start of
each loop iteration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912031112.1814574-3-sh_def@163.com?part=1