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

[sashiko-bot@kernel.org]

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