Re: [PATCH v7 2/7] perf test: Provide setup for the shell test suite

sashiko-bot
2026-04-16T12:07:09Z

Sashiko AI review found 1 potential issue(s):
- [Medium] Unescaped directory paths passed to `system()` enable arbitrary command execution and break the test suite if the path contains spaces.
--

commit e6010a1e8737e6a67dc7b0bb3ea8439fbf49269b
Author: Jakub Brnak <jbrnak@redhat.com>

perf test: Provide setup for the shell test suite

This commit adds support for running a setup script before executing test cases
in the perftool-testsuite. It checks for the existence of "setup.sh" and skips
the test suite if the setup fails.

> diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
> index 680dfe650f146..33f1e46267f41 100644
> --- a/tools/perf/tests/tests-scripts.c
> +++ b/tools/perf/tests/tests-scripts.c

[ ... ]

> @@ -255,6 +262,23 @@ static int shell_test__run(struct test_suite *test, int subtest)
> else /* Single test case */
> file = test->test_cases[0].name;
>
> + /* Run setup if needed */
> + if (test_info->has_setup == RUN_SETUP) {
> + char *setup_script;
> +
> + if (asprintf(&setup_script, "%s%s%s", test_info->base_path,
> + SHELL_SETUP, verbose ? " -v" : "") < 0)
> + return TEST_SETUP_FAIL;
> +
> + err = system(setup_script);

Does this code handle directory paths with spaces correctly?

If test_info->base_path contains a space (for example, in "/tmp/perf test/"),
system() will evaluate the string using /bin/sh -c without quotes around the
path, which breaks the test suite.

Could this also allow arbitrary command execution if shell metacharacters
are present in the path?

> + free(setup_script);
> +
> + if (err)
> + return TEST_SETUP_FAIL;
> + } else if (test_info->has_setup == FAILED_SETUP) {
> + return TEST_SKIP; /* Skip test suite if setup failed */
> + }

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260416111419.385010-1-jbrnak@redhat.com?part=2