Re: [PATCH v2 08/14] 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] Modifying the global `summary_failed_tests_buf` via `strbuf_addf()` introduces a heap deadlock and Use-After-Free vulnerability due to signal-interrupted cleanup via `siglongjmp`.
--

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

perf test: Add summary reporting

This commit introduces an automated global execution summary printed at the
end of a perf test run. It tracks test completion metrics and accumulates
the descriptions of all failed test cases into a global string buffer for
reporting.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -390,6 +401,15 @@ static int print_test_result(struct test_suite *t, int curr_suite, int curr_test
>  		break;
>  	case TEST_FAIL:
>  	default:
> +		summary_tests_failed++;
> +		if (test_suite__num_test_cases(t) > 1)
> +			strbuf_addf(&summary_failed_tests_buf, "  %3d.%1d: %s\n",
> +				    curr_suite + 1, curr_test_case + 1,
> +				    test_description(t, curr_test_case));
> +		else
> +			strbuf_addf(&summary_failed_tests_buf, "  %3d: %s\n",
> +				    curr_suite + 1,
> +				    test_description(t, curr_test_case));

[Severity: Medium]
Is it safe to mutate the global summary_failed_tests_buf here using
strbuf_addf()?

The perf test harness installs a signal handler for SIGINT/SIGTERM
(cmd_test_sig_handler) that abruptly exits the current execution frame via
siglongjmp().

If a user interrupts the tool precisely when strbuf_addf() is executing, the
siglongjmp() will abandon the call mid-flight. Because strbuf_addf() relies
on realloc(), which is not async-signal-safe, this could leave the heap lock
acquired and the global buffer in a corrupted state.

The cleanup path at err_out then unconditionally calls print_tests_summary(),
which accesses and frees the buffer:

print_tests_summary() {
    ...
    pr_info("%s", summary_failed_tests_buf.buf);
    strbuf_release(&summary_failed_tests_buf);
}

Could this lead to a deadlock or a use-after-free crash during the emergency
cleanup?

>  		color_fprintf(stderr, PERF_COLOR_RED, " FAILED!\n");
>  		break;
>  	}

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