Re: [PATCH 09/15] perf test: Add summary reporting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a Use-After-Free and Double Free vulnerability by attempting to read and release a dynamically allocated string buffer (`summary_failed_tests_buf`) on the emergency signal cleanup path.
--

commit f5ac11752bd41259824d926b27193d632a097f85
Author: Ian Rogers <irogers@google.com>

perf test: Add summary reporting

This commit introduces an automated global execution summary printed at the
tail of the test run, tracking passed, skipped, and failed test cases,
along with descriptions of failed tests.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index e00b0bffa67cf..336d24105c9f9 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -963,6 +982,23 @@ static void cmd_test_sig_handler(int sig)
>  	siglongjmp(cmd_test_jmp_buf, sig);
>  }
>  
> +static void print_tests_summary(void)
> +{
> +	pr_info("\n=== Test Summary ===\n");
> +	pr_info("Passed main tests : %u\n", summary_tests_passed);
> +	pr_info("Passed subtests   : %u\n", summary_subtests_passed);
> +	pr_info("Skipped tests     : %u\n", summary_tests_skipped);
> +	if (summary_tests_failed > 0) {
> +		color_fprintf(stderr, PERF_COLOR_RED, "Failed tests      : %u\n",
> +			      summary_tests_failed);
> +		pr_info("List of failed tests:\n");
> +		pr_info("%s", summary_failed_tests_buf.buf);
> +	} else {
> +		color_fprintf(stderr, PERF_COLOR_GREEN, "Failed tests      : 0\n");
> +	}
> +	strbuf_release(&summary_failed_tests_buf);
> +}
> +
>  static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  		      struct intlist *skiplist)
[ ... ]
> @@ -1075,6 +1114,7 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  		for (size_t x = 0; x < num_tests; x++)
>  			finish_test(child_tests, x, num_tests, width);
>  	}
> +	print_tests_summary();

[Severity: Medium]
Can this sequence lead to a use-after-free and double-free if the test suite
is interrupted by a signal?

If a signal like SIGINT arrives while print_test_result() is dynamically
reallocating the string buffer via strbuf_addf():

print_test_result() {
        ...
        strbuf_addf(&summary_failed_tests_buf, "  %3d.%1d: %s\n", ...);
        ...
}

The signal handler cmd_test_sig_handler() uses siglongjmp() to abort
execution and jumps directly to the err_out path in __cmd_test().

__cmd_test() {
...
err_out:
        ...
        if (err) {
                ...
        }
        print_tests_summary();
...
}

If realloc() had already freed the old memory block but the signal
interrupted execution before the summary_failed_tests_buf.buf pointer could
be updated, would print_tests_summary() dereference a freed pointer when
printing the summary, and then double-free it during strbuf_release()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601000548.1195604-1-irogers@google.com?part=9