Re: [PATCH v2 1/2] perf test: Add per-test timeout support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf test: TOCTOU race in signal handler can kill unintended process groups
--

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

perf test: Add per-test timeout support

When a test hangs in perf test, the entire test run hangs and makes it
difficult to inspect output from other tests. Add a new TEST_TIMEOUT result
category and a per-test timeout.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index d2f594921e25b..4ecbaf757095e 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -1096,7 +1254,10 @@ static int finish_tests_parallel(struct child_test **child_tests, size_t num_tes
>  					close(child->process.err);
>  					child->process.err = -1;
>  				}
> +				kill_timed_out_group(child);
>  				child->result = finish_command(&child->process);
> +				if (child->timed_out)
> +					child->result = TEST_TIMEOUT;
>  				child->process.pid = 0;

[Severity: Medium]
Is there a TOCTOU race window here in finish_tests_parallel() before
child->process.pid is zeroed?

If check_if_command_finished() falls back to waitpid(..., WNOHANG) (e.g. when
/proc is unmounted) and reaps the child, finish_command() won't block.

If cmd_test_sig_handler is triggered via SIGINT or SIGTERM right after
finish_command() reaps the child but before child->process.pid is zeroed,
the asynchronous signal handler jumps back to __cmd_test() and executes
a kill on the stale PID.

[ ... ]
> @@ -1327,12 +1507,27 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  			pr_debug3("Killing %d pid %d\n",
>  				  child_test->suite_num + 1,
>  				  child_test->process.pid);
> +			kill(-child_test->process.pid, err);
>  			kill(child_test->process.pid, err);
>  		}
>  		goto err_out;
>  	}

[Severity: Medium]
Can this escalation from kill(pid) to kill(-pid) in __cmd_test() turn the
single-process race into a full process group kill?

If the race window described above occurs, and a concurrent high-rate fork
workload wraps the PID space exactly to the newly reaped PID, this kill(-pid)
command could wipe out an entire unrelated process group rather than just a
single reused PID.

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