[PATCH v7 0/7] Introduce structure for shell tests

[PATCH v7 0/7] Introduce structure for shell tests

[Jakub Brnak]

Hi,

Sending the next revision addressing review feedback from Ian and
Namhyung on v6.

Thanks,
Jakub

Link to v6: https://lore.kernel.org/linux-perf-users/20260309155728.98026-1-jbrnak@redhat.com/

Changes since v6:

Patch 1: fixed fd leaks and missing error checks, added documentation
for the shell test infrastructure (requested by Namhyung)

Patch 2: fixed race condition by marking setup suites exclusive, added
NULL fallback for setup.sh description, fixed indentation

Patch 4: replaced VLA with calloc, fixed delete_file to not abort
nftw on failure, added comment about intentional log retention on error

Patch 5: fixed premature loop termination on dots in test name

Jakub Brnak (7):
  perf tests: Create a structure for shell tests
  perf test: Provide setup for the shell test suite
  perf test: Add empty setup for base_probe
  perf test: Introduce storing logs for shell tests
  perf test: Format log directories for shell tests
  perf test: Remove perftool drivers
  perf test: Fix relative path for 'stderr-whitelist.txt'

 tools/perf/tests/builtin-test.c               | 175 ++++++++-
 tools/perf/tests/shell/base_probe/setup.sh    |  13 +
 .../tests/shell/base_report/test_basic.sh     |   6 +-
 .../tests/shell/perftool-testsuite_probe.sh   |  24 --
 .../tests/shell/perftool-testsuite_report.sh  |  23 --
 tools/perf/tests/tests-scripts.c              | 359 +++++++++++++++---
 tools/perf/tests/tests-scripts.h              |  17 +
 tools/perf/tests/tests.h                      |   8 +-
 8 files changed, 513 insertions(+), 112 deletions(-)
 create mode 100755 tools/perf/tests/shell/base_probe/setup.sh
 delete mode 100755 tools/perf/tests/shell/perftool-testsuite_probe.sh
 delete mode 100755 tools/perf/tests/shell/perftool-testsuite_report.sh

-- 
2.52.0

[PATCH v7 1/7] perf tests: Create a structure for shell tests

[Jakub Brnak]

The general structure of test suites with test cases has been implemented
for C tests for some time, while shell tests were just all put into a list
without any possible structuring.

Provide the same possibility of test suite structure for shell tests. The
suite is created for each subdirectory located in the 'perf/tests/shell'
directory that contains at least one test script. All of the deeper levels
of subdirectories will be merged with the first level of test cases.
The name of the test suite is the name of the subdirectory, where the test
cases are located. For all of the test scripts that are not in any
subdirectory, a test suite with a single test case is created as it has
been till now.

The new structure of the shell tests for 'perf test list':
    77: build id cache operations
    78: coresight
    78:1: CoreSight / ASM Pure Loop
    78:2: CoreSight / Memcpy 16k 10 Threads
    78:3: CoreSight / Thread Loop 10 Threads - Check TID
    78:4: CoreSight / Thread Loop 2 Threads - Check TID
    78:5: CoreSight / Unroll Loop Thread 10
    79: daemon operations
    80: perf diff tests

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/tests-scripts.c | 319 ++++++++++++++++++++++++++-----
 tools/perf/tests/tests-scripts.h |   5 +
 2 files changed, 277 insertions(+), 47 deletions(-)

diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index f18c4cd337c8..680dfe650f14 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -1,4 +1,53 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Shell test suite infrastructure for perf test.
+ *
+ * Shell tests live under tools/perf/tests/shell/. The framework supports two
+ * layouts:
+ *
+ * 1) Standalone scripts: A .sh file placed directly in the shell/ directory
+ *    becomes a test suite with a single test case. The suite name is taken
+ *    from the script's description line (the second line, after the shebang).
+ *    Example: shell/record.sh -> "perf record tests"
+ *
+ * 2) Subdirectory suites: A subdirectory under shell/ containing .sh test
+ *    scripts becomes a test suite with multiple test cases. All scripts in
+ *    the subdirectory (and deeper levels) are merged into one suite. The
+ *    suite name defaults to the directory name.
+ *    Example: shell/coresight/ -> suite "coresight" with N test cases
+ *
+ * Setup scripts:
+ *    If a subdirectory contains a "setup.sh" file, it is executed before the
+ *    first test case. The description line of setup.sh overrides the suite
+ *    name. If setup fails, all test cases in the suite are skipped. Suites
+ *    with a setup script are marked exclusive (run sequentially, not in
+ *    parallel) to avoid interference between setup and test execution.
+ *    setup.sh is filtered out of the test case list -- it is not a test.
+ *
+ * Log directories:
+ *    For subdirectory suites, temporary log directories are created under
+ *    /tmp/perf_test_<sanitized_name>.XXXXXX where non-alphanumeric characters
+ *    in the suite name are replaced with underscores. The path is exported as
+ *    PERFSUITE_RUN_DIR. Test scripts can use this variable for storing
+ *    intermediate files and logs. Logs are removed after the suite completes
+ *    unless PERFTEST_KEEP_LOGS=y is set in the environment. On test harness
+ *    errors, logs are kept for debugging.
+ *
+ * Script conventions:
+ *    - Scripts must have a .sh extension and be executable.
+ *    - Scripts must start with a #! shebang (typically #!/bin/bash).
+ *    - The second line (after leading # and whitespace are stripped) is the
+ *      description shown by 'perf test list'.
+ *    - Appending " (exclusive)" to the description marks a test for
+ *      sequential execution.
+ *    - Exit code 0 = pass, 2 = skip, anything else = fail.
+ *
+ * Helper libraries:
+ *    shell/common/  - Shared initialization (init.sh), settings (settings.sh),
+ *                     pattern matching helpers, and result reporting functions.
+ *    shell/lib/     - Reusable shell and Python helpers for specific features
+ *                     (coresight, probes, stat output parsing, etc.).
+ */
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -151,14 +200,64 @@ static char *strdup_check(const char *str)
 	return newstr;
 }
 
-static int shell_test__run(struct test_suite *test, int subtest __maybe_unused)
+/* Check if dirent entry is a directory using file descriptor, handling DT_UNKNOWN case */
+static bool is_directory_fd(int dir_fd, struct dirent *ent)
 {
-	const char *file = test->priv;
+	struct stat st;
+
+	if (ent->d_type == DT_DIR)
+		return true;
+
+	if (ent->d_type != DT_UNKNOWN)
+		return false;
+
+	/* Need to stat the file to determine if it's a directory */
+	if (fstatat(dir_fd, ent->d_name, &st, 0) != 0)
+		return false;
+
+	return S_ISDIR(st.st_mode);
+}
+
+/* Free the whole structure of test_suite with its test_cases */
+static void free_suite(struct test_suite *suite)
+{
+	if (suite->test_cases) {
+		int num = 0;
+
+		while (suite->test_cases[num].name) { /* Last case has name set to NULL */
+			free((void *) suite->test_cases[num].name);
+			free((void *) suite->test_cases[num].desc);
+			num++;
+		}
+		free(suite->test_cases);
+	}
+	if (suite->desc)
+		free((void *) suite->desc);
+	if (suite->priv) {
+		struct shell_test_info *test_info = suite->priv;
+
+		free((void *) test_info->base_path);
+		free(test_info);
+	}
+
+	free(suite);
+}
+
+static int shell_test__run(struct test_suite *test, int subtest)
+{
+	const char *file;
 	int err;
 	char *cmd = NULL;
 
+	/* Get absolute file path */
+	if (subtest >= 0)
+		file = test->test_cases[subtest].name;
+	else	/* Single test case */
+		file = test->test_cases[0].name;
+
 	if (asprintf(&cmd, "%s%s", file, verbose ? " -v" : "") < 0)
 		return TEST_FAIL;
+
 	err = system(cmd);
 	free(cmd);
 	if (!err)
@@ -167,65 +266,117 @@ static int shell_test__run(struct test_suite *test, int subtest __maybe_unused)
 	return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL;
 }
 
-static void append_script(int dir_fd, const char *name, char *desc,
-			  struct test_suite ***result,
-			  size_t *result_sz)
+static struct test_suite *prepare_test_suite(int dir_fd)
 {
-	char filename[PATH_MAX], link[128];
-	struct test_suite *test_suite, **result_tmp;
-	struct test_case *tests;
+	char dirpath[PATH_MAX], link[128];
 	ssize_t len;
-	char *exclusive;
+	struct test_suite *test_suite = NULL;
+	struct shell_test_info *test_info;
 
+	/* Get dir absolute path */
 	snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
-	len = readlink(link, filename, sizeof(filename));
+	len = readlink(link, dirpath, sizeof(dirpath));
 	if (len < 0) {
 		pr_err("Failed to readlink %s", link);
-		return;
+		return NULL;
 	}
-	filename[len++] = '/';
-	strcpy(&filename[len], name);
+	dirpath[len++] = '/';
+	dirpath[len] = '\0';
 
-	tests = calloc(2, sizeof(*tests));
-	if (!tests) {
-		pr_err("Out of memory while building script test suite list\n");
-		return;
-	}
-	tests[0].name = strdup_check(name);
-	exclusive = strstr(desc, " (exclusive)");
-	if (exclusive != NULL) {
-		tests[0].exclusive = true;
-		exclusive[0] = '\0';
-	}
-	tests[0].desc = strdup_check(desc);
-	tests[0].run_case = shell_test__run;
 	test_suite = zalloc(sizeof(*test_suite));
 	if (!test_suite) {
 		pr_err("Out of memory while building script test suite list\n");
-		free(tests);
-		return;
+		return NULL;
 	}
-	test_suite->desc = desc;
-	test_suite->test_cases = tests;
-	test_suite->priv = strdup_check(filename);
+
+	test_info = zalloc(sizeof(*test_info));
+	if (!test_info) {
+		pr_err("Out of memory while building script test suite list\n");
+		free(test_suite);
+		return NULL;
+	}
+
+	test_info->base_path = strdup_check(dirpath);		/* Absolute path to dir */
+
+	test_suite->priv = test_info;
+	test_suite->desc = NULL;
+	test_suite->test_cases = NULL;
+
+	return test_suite;
+}
+
+static void append_suite(struct test_suite ***result,
+			  size_t *result_sz, struct test_suite *test_suite)
+{
+	struct test_suite **result_tmp;
+
 	/* Realloc is good enough, though we could realloc by chunks, not that
 	 * anyone will ever measure performance here */
 	result_tmp = realloc(*result, (*result_sz + 1) * sizeof(*result_tmp));
 	if (result_tmp == NULL) {
 		pr_err("Out of memory while building script test suite list\n");
-		free(tests);
-		free(test_suite);
+		free_suite(test_suite);
 		return;
 	}
+
 	/* Add file to end and NULL terminate the struct array */
 	*result = result_tmp;
 	(*result)[*result_sz] = test_suite;
 	(*result_sz)++;
 }
 
-static void append_scripts_in_dir(int dir_fd,
-				  struct test_suite ***result,
-				  size_t *result_sz)
+static void append_script_to_suite(int dir_fd, const char *name, char *desc,
+					struct test_suite *test_suite, size_t *tc_count)
+{
+	char file_name[PATH_MAX], link[128];
+	struct test_case *tests;
+	ssize_t len;
+	char *exclusive;
+
+	if (!test_suite) {
+		free(desc);
+		return;
+	}
+
+	/* Requires an empty test case at the end */
+	tests = realloc(test_suite->test_cases, (*tc_count + 2) * sizeof(*tests));
+	if (!tests) {
+		pr_err("Out of memory while building script test suite list\n");
+		free(desc);
+		return;
+	}
+	test_suite->test_cases = tests;
+	tests[*tc_count].name = NULL;	/* Ensure termination for early returns */
+
+	/* Get path to the test script */
+	snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
+	len = readlink(link, file_name, sizeof(file_name));
+	if (len < 0) {
+		pr_err("Failed to readlink %s", link);
+		free(desc);
+		return;
+	}
+	file_name[len++] = '/';
+	strcpy(&file_name[len], name);
+
+	/* Get path to the script from base dir */
+	tests[(*tc_count)].name = strdup_check(file_name);
+	tests[(*tc_count)].exclusive = false;
+	exclusive = strstr(desc, " (exclusive)");
+	if (exclusive != NULL) {
+		tests[(*tc_count)].exclusive = true;
+		exclusive[0] = '\0';
+	}
+	tests[(*tc_count)].desc = desc;
+	tests[(*tc_count)].skip_reason = NULL;	/* Unused */
+	tests[(*tc_count)++].run_case = shell_test__run;
+
+	tests[(*tc_count)].name = NULL;		/* End the test cases */
+}
+
+static void append_scripts_in_subdir(int dir_fd,
+				  struct test_suite *suite,
+				  size_t *tc_count)
 {
 	struct dirent **entlist;
 	struct dirent *ent;
@@ -244,22 +395,96 @@ static void append_scripts_in_dir(int dir_fd,
 			char *desc = shell_test__description(dir_fd, ent->d_name);
 
 			if (desc) /* It has a desc line - valid script */
-				append_script(dir_fd, ent->d_name, desc, result, result_sz);
+				append_script_to_suite(dir_fd, ent->d_name, desc, suite, tc_count);
 			continue;
 		}
-		if (ent->d_type != DT_DIR) {
-			struct stat st;
-
-			if (ent->d_type != DT_UNKNOWN)
-				continue;
-			fstatat(dir_fd, ent->d_name, &st, 0);
-			if (!S_ISDIR(st.st_mode))
-				continue;
+
+		if (!is_directory_fd(dir_fd, ent))
+			continue;
+
+		fd = openat(dir_fd, ent->d_name, O_PATH);
+		if (fd < 0)
+			continue;
+
+		/* Recurse into the dir */
+		append_scripts_in_subdir(fd, suite, tc_count);
+		close(fd);
+	}
+	for (i = 0; i < n_dirs; i++) /* Clean up */
+		zfree(&entlist[i]);
+	free(entlist);
+}
+
+static void append_suites_in_dir(int dir_fd,
+				  struct test_suite ***result,
+				  size_t *result_sz)
+{
+	struct dirent **entlist;
+	struct dirent *ent;
+	int n_dirs, i;
+
+	/* List files, sorted by alpha */
+	n_dirs = scandirat(dir_fd, ".", &entlist, NULL, alphasort);
+	if (n_dirs == -1)
+		return;
+	for (i = 0; i < n_dirs && (ent = entlist[i]); i++) {
+		int fd;
+		struct test_suite *test_suite;
+		size_t cases_count = 0;
+
+		if (ent->d_name[0] == '.')
+			continue; /* Skip hidden files */
+		if (is_test_script(dir_fd, ent->d_name)) { /* It's a test */
+			char *desc = shell_test__description(dir_fd, ent->d_name);
+
+			if (desc) { /* It has a desc line - valid script */
+				/* Create a test suite with a single test case */
+				test_suite = prepare_test_suite(dir_fd);
+				if (!test_suite) {
+					free(desc);
+					continue;
+				}
+				append_script_to_suite(dir_fd, ent->d_name, desc,
+							       test_suite, &cases_count);
+
+				if (cases_count) {
+					test_suite->desc = strdup_check(
+							test_suite->test_cases[0].desc);
+					append_suite(result, result_sz, test_suite);
+				} else { /* Wasn't able to create the test case */
+					free_suite(test_suite);
+				}
+			}
+			continue;
 		}
+
+		if (!is_directory_fd(dir_fd, ent))
+			continue;
+
 		if (strncmp(ent->d_name, "base_", 5) == 0)
 			continue; /* Skip scripts that have a separate driver. */
+
+		/* Scan subdir for test cases */
 		fd = openat(dir_fd, ent->d_name, O_PATH);
-		append_scripts_in_dir(fd, result, result_sz);
+		if (fd < 0)
+			continue;
+		test_suite = prepare_test_suite(fd);	/* Prepare a testsuite with its path */
+		if (!test_suite) {
+			close(fd);
+			continue;
+		}
+
+		append_scripts_in_subdir(fd, test_suite, &cases_count);
+		if (cases_count == 0) {
+			free_suite(test_suite);
+			close(fd);
+			continue;
+		}
+
+		/* If no setup, set name to the directory */
+		test_suite->desc = strdup_check(ent->d_name);
+
+		append_suite(result, result_sz, test_suite);
 		close(fd);
 	}
 	for (i = 0; i < n_dirs; i++) /* Clean up */
@@ -278,7 +503,7 @@ struct test_suite **create_script_test_suites(void)
 	 * length array.
 	 */
 	if (dir_fd >= 0)
-		append_scripts_in_dir(dir_fd, &result, &result_sz);
+		append_suites_in_dir(dir_fd, &result, &result_sz);
 
 	result_tmp = realloc(result, (result_sz + 1) * sizeof(*result_tmp));
 	if (result_tmp == NULL) {
diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h
index b553ad26ea17..1b849d4e70f4 100644
--- a/tools/perf/tests/tests-scripts.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -4,6 +4,11 @@
 
 #include "tests.h"
 
+/* Additional information attached to shell tests */
+struct shell_test_info {
+	const char *base_path;
+};
+
 struct test_suite **create_script_test_suites(void);
 
 #endif /* TESTS_SCRIPTS_H */
-- 
2.52.0

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

[Jakub Brnak]

Some of the perftool-testsuite test cases require a setup to be done
beforehand as may be recording data, setting up cache or restoring sample
rate. The setup file also provides the possibility to set the name of
the test suite, if the name of the directory is not good enough.

Check for the existence of the "setup.sh" script for the shell test
suites and run it before the any of the test cases. If the setup fails,
skip all of the test cases of the test suite as the setup may be
required for the result to be valid.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/builtin-test.c  | 30 ++++++++++++++++++-----
 tools/perf/tests/tests-scripts.c | 41 +++++++++++++++++++++++++++++---
 tools/perf/tests/tests-scripts.h | 11 ++++++++-
 tools/perf/tests/tests.h         |  8 ++++---
 4 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 06507066213b..76fc37b440be 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -264,6 +264,22 @@ static test_fnptr test_function(const struct test_suite *t, int test_case)
 	return t->test_cases[test_case].run_case;
 }
 
+/* If setup fails, skip all test cases */
+static void check_shell_setup(const struct test_suite *t, int ret)
+{
+	struct shell_test_info *test_info;
+
+	if (!t->priv)
+		return;
+
+	test_info = t->priv;
+
+	if (ret == TEST_SETUP_FAIL)
+		test_info->has_setup = FAILED_SETUP;
+	else if (test_info->has_setup == RUN_SETUP)
+		test_info->has_setup = PASSED_SETUP;
+}
+
 static bool test_exclusive(const struct test_suite *t, int test_case)
 {
 	if (test_case <= 0)
@@ -353,10 +369,9 @@ static int run_test_child(struct child_process *process)
 	return -err;
 }
 
-#define TEST_RUNNING -3
-
-static int print_test_result(struct test_suite *t, int curr_suite, int curr_test_case,
-			     int result, int width, int running)
+static int print_test_result(struct test_suite *t, int curr_suite,
+			     int curr_test_case, int result, int width,
+			     int running)
 {
 	if (test_suite__num_test_cases(t) > 1) {
 		int subw = width > 2 ? width - 2 : width;
@@ -373,7 +388,8 @@ static int print_test_result(struct test_suite *t, int curr_suite, int curr_test
 	case TEST_OK:
 		pr_info(" Ok\n");
 		break;
-	case TEST_SKIP: {
+	case TEST_SKIP:
+	case TEST_SETUP_FAIL: {
 		const char *reason = skip_reason(t, curr_test_case);
 
 		if (reason)
@@ -488,6 +504,7 @@ static void finish_test(struct child_test **child_tests, int running_test, int c
 	}
 	/* Clean up child process. */
 	ret = finish_command(&child_test->process);
+	check_shell_setup(t, ret);
 	if (verbose > 1 || (verbose == 1 && ret == TEST_FAIL))
 		fprintf(stderr, "%s", err_output.buf);
 
@@ -510,7 +527,8 @@ static int start_test(struct test_suite *test, int curr_suite, int curr_test_cas
 			err = test_function(test, curr_test_case)(test, curr_test_case);
 			pr_debug("---- end ----\n");
 			print_test_result(test, curr_suite, curr_test_case, err, width,
-					  /*running=*/0);
+				  /*running=*/0);
+			check_shell_setup(test, err);
 		}
 		return 0;
 	}
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index 680dfe650f14..33f1e46267f4 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -187,6 +187,12 @@ static bool is_test_script(int dir_fd, const char *name)
 	return is_shell_script(dir_fd, name);
 }
 
+/* Filter for scandir */
+static int setup_filter(const struct dirent *entry)
+{
+	return strcmp(entry->d_name, SHELL_SETUP);
+}
+
 /* Duplicate a string and fall over and die if we run out of memory */
 static char *strdup_check(const char *str)
 {
@@ -245,6 +251,7 @@ static void free_suite(struct test_suite *suite)
 
 static int shell_test__run(struct test_suite *test, int subtest)
 {
+	struct shell_test_info *test_info = test->priv;
 	const char *file;
 	int err;
 	char *cmd = NULL;
@@ -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);
+		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 */
+	}
+
 	if (asprintf(&cmd, "%s%s", file, verbose ? " -v" : "") < 0)
 		return TEST_FAIL;
 
@@ -297,6 +321,7 @@ static struct test_suite *prepare_test_suite(int dir_fd)
 	}
 
 	test_info->base_path = strdup_check(dirpath);		/* Absolute path to dir */
+	test_info->has_setup = NO_SETUP;
 
 	test_suite->priv = test_info;
 	test_suite->desc = NULL;
@@ -383,7 +408,7 @@ static void append_scripts_in_subdir(int dir_fd,
 	int n_dirs, i;
 
 	/* List files, sorted by alpha */
-	n_dirs = scandirat(dir_fd, ".", &entlist, NULL, alphasort);
+	n_dirs = scandirat(dir_fd, ".", &entlist, setup_filter, alphasort);
 	if (n_dirs == -1)
 		return;
 	for (i = 0; i < n_dirs && (ent = entlist[i]); i++) {
@@ -481,8 +506,18 @@ static void append_suites_in_dir(int dir_fd,
 			continue;
 		}
 
-		/* If no setup, set name to the directory */
-		test_suite->desc = strdup_check(ent->d_name);
+		if (is_test_script(fd, SHELL_SETUP)) {	/* Check for setup existence */
+			char *desc = shell_test__description(fd, SHELL_SETUP);
+
+			/* Set the suite name by the setup description */
+			test_suite->desc = desc ? desc : strdup_check(ent->d_name);
+			((struct shell_test_info *)(test_suite->priv))->has_setup = RUN_SETUP;
+			for (size_t j = 0; j < cases_count; j++)
+				test_suite->test_cases[j].exclusive = true;
+		} else {
+			/* If no setup, set name to the directory */
+			test_suite->desc = strdup_check(ent->d_name);
+		}
 
 		append_suite(result, result_sz, test_suite);
 		close(fd);
diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h
index 1b849d4e70f4..013031239883 100644
--- a/tools/perf/tests/tests-scripts.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -4,9 +4,18 @@
 
 #include "tests.h"
 
-/* Additional information attached to shell tests */
+#define SHELL_SETUP "setup.sh"
+
+enum shell_setup {
+	NO_SETUP     = 0,
+	RUN_SETUP    = 1,
+	FAILED_SETUP = 2,
+	PASSED_SETUP = 3,
+};
+
 struct shell_test_info {
 	const char *base_path;
+	enum shell_setup has_setup;
 };
 
 struct test_suite **create_script_test_suites(void);
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index f5f1238d1f7f..d8d4156e31de 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -6,9 +6,11 @@
 #include "util/debug.h"
 
 enum {
-	TEST_OK   =  0,
-	TEST_FAIL = -1,
-	TEST_SKIP = -2,
+	TEST_OK         =  0,
+	TEST_FAIL       = -1,
+	TEST_SKIP       = -2,
+	TEST_RUNNING    = -3,
+	TEST_SETUP_FAIL = -4,
 };
 
 #define TEST_ASSERT_VAL(text, cond)					 \
-- 
2.52.0

[PATCH v7 3/7] perf test: Add empty setup for base_probe

[Jakub Brnak]

Add empty setup to set a proper name for base_probe testsuite, can be
utilized for basic test setup for the future.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/shell/base_probe/setup.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100755 tools/perf/tests/shell/base_probe/setup.sh

diff --git a/tools/perf/tests/shell/base_probe/setup.sh b/tools/perf/tests/shell/base_probe/setup.sh
new file mode 100755
index 000000000000..fbb99325b555
--- /dev/null
+++ b/tools/perf/tests/shell/base_probe/setup.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+# perftool-testsuite :: perf_probe
+# SPDX-License-Identifier: GPL-2.0
+
+#
+#	setup.sh of perf probe test
+#	Author: Michael Petlan <mpetlan@redhat.com>
+#
+#	Description:
+#
+#		Setting testsuite name, for future use
+#
+#
-- 
2.52.0

[PATCH v7 4/7] perf test: Introduce storing logs for shell tests

[Jakub Brnak]

Create temporary directories for storing log files for shell tests
that could help while debugging. The log files are necessary for
perftool testsuite test cases also. If the variable PERFTEST_KEEP_LOGS
is set keep the logs, else delete them.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/builtin-test.c  | 133 ++++++++++++++++++++++++++++++-
 tools/perf/tests/tests-scripts.c |   4 +
 tools/perf/tests/tests-scripts.h |   3 +
 3 files changed, 136 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 76fc37b440be..306ecfe5e22f 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -6,6 +6,7 @@
  */
 #include <ctype.h>
 #include <fcntl.h>
+#include <ftw.h>
 #include <errno.h>
 #ifdef HAVE_BACKTRACE_SUPPORT
 #include <execinfo.h>
@@ -51,6 +52,9 @@ static unsigned int runs_per_test = 1;
 const char *dso_to_test;
 const char *test_objdump_path = "objdump";
 
+/* Number of logged test suites. */
+size_t num_logged_suites;
+
 /*
  * List of architecture specific tests. Not a weak symbol as the array length is
  * dependent on the initialization, as such GCC with LTO complains of
@@ -288,6 +292,91 @@ static bool test_exclusive(const struct test_suite *t, int test_case)
 	return t->test_cases[test_case].exclusive;
 }
 
+static int delete_file(const char *fpath, const struct stat *sb __maybe_unused,
+		       int typeflag, struct FTW *ftwbuf)
+{
+	/* Stop traversal if going too deep */
+	if (ftwbuf->level > 5) {
+		pr_err("Tree traversal reached level %d, stopping.", ftwbuf->level);
+		return 1;
+	}
+
+	/* Remove only expected directories */
+	if (typeflag == FTW_D || typeflag == FTW_DP) {
+		const char *dirname = fpath + ftwbuf->base;
+
+		if (strcmp(dirname, "logs") && strcmp(dirname, "examples") &&
+		    strcmp(dirname, "header_tar") && strncmp(dirname, "perf_", 5)) {
+			pr_err("Unknown directory %s", dirname);
+			return 0;
+		}
+	}
+
+	/* Attempt to remove the file, continue on failure */
+	if (remove(fpath))
+		pr_err("Failed to remove file: %s", fpath);
+
+	return 0;
+}
+
+static bool create_logs(struct test_suite *t, int pass)
+{
+	bool store_logs = t->priv && ((struct shell_test_info *)(t->priv))->store_logs;
+	bool exclusive = false;
+	int tc;
+
+	test_suite__for_each_test_case(t, tc) {
+		if (test_exclusive(t, tc)) {
+			exclusive = true;
+			break;
+		}
+	}
+
+	if (pass == 1 && (!exclusive || sequential || dont_fork)) {
+		/* Sequential and non-exclusive tests run on the first pass. */
+		return store_logs;
+	} else if (pass != 1 && exclusive && !sequential && !dont_fork) {
+		/* Exclusive tests without sequential run on the second pass. */
+		return store_logs;
+	}
+	return false;
+}
+
+static char *setup_shell_logs(const char *name)
+{
+	char template[PATH_MAX];
+	char *temp_dir;
+
+	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", name) >= PATH_MAX) {
+		pr_err("Failed to create log dir template");
+		return NULL; /* Skip the testsuite */
+	}
+
+	temp_dir = mkdtemp(template);
+	if (temp_dir) {
+		setenv("PERFSUITE_RUN_DIR", temp_dir, 1);
+		return strdup(temp_dir);
+	}
+
+	pr_err("Failed to create the temporary directory");
+
+	return NULL; /* Skip the testsuite */
+}
+
+static void cleanup_shell_logs(char *dirname)
+{
+	char *keep_logs = getenv("PERFTEST_KEEP_LOGS");
+
+	/* Check if logs should be kept or do cleanup */
+	if (dirname) {
+		if (!keep_logs || strcmp(keep_logs, "y") != 0)
+			nftw(dirname, delete_file, 8, FTW_DEPTH | FTW_PHYS);
+		free(dirname);
+	}
+
+	unsetenv("PERFSUITE_RUN_DIR");
+}
+
 static bool perf_test__matches(const char *desc, int suite_num, int argc, const char *argv[])
 {
 	int i;
@@ -627,9 +716,19 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
 	 * runs the exclusive tests sequentially. In other modes all tests are
 	 * run in pass 1.
 	 */
+
 	for (int pass = 1; pass <= 2; pass++) {
 		int child_test_num = 0;
 		int curr_suite = 0;
+		size_t tmpdir_sz = num_logged_suites * runs_per_test;
+		char **tmpdir = tmpdir_sz ? calloc(tmpdir_sz, sizeof(*tmpdir)) : NULL;
+		unsigned int logged_suites = 0;
+
+		if (tmpdir_sz && !tmpdir) {
+			pr_err("Out of memory while allocating log directories\n");
+			err = -ENOMEM;
+			goto err_out;
+		}
 
 		for (struct test_suite **t = suites; *t; t++, curr_suite++) {
 			int curr_test_case;
@@ -663,16 +762,37 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
 			}
 
 			for (unsigned int run = 0; run < runs_per_test; run++) {
+				/* Setup temporary log directories for shell test suites */
+				if (create_logs(*t, pass)) {
+					struct shell_test_info *info = (*t)->priv;
+
+					tmpdir[logged_suites] = setup_shell_logs((*t)->desc);
+					/* Couldn't create log dir, skip test suite */
+					if (tmpdir[logged_suites] == NULL) {
+						if (info->has_setup != NO_SETUP)
+							info->has_setup = FAILED_SETUP;
+					} else {
+						logged_suites++;
+						if (info->has_setup == PASSED_SETUP ||
+						    info->has_setup == FAILED_SETUP)
+							info->has_setup = RUN_SETUP;
+					}
+				}
+
 				test_suite__for_each_test_case(*t, curr_test_case) {
 					if (!suite_matched &&
 					    !perf_test__matches(test_description(*t, curr_test_case),
 								curr_suite, argc, argv))
 						continue;
-					err = start_test(*t, curr_suite, curr_test_case,
-							 &child_tests[child_test_num++],
-							 width, pass);
-					if (err)
+				err = start_test(*t, curr_suite, curr_test_case,
+						 &child_tests[child_test_num++],
+						 width, pass);
+					if (err) {
+						for (size_t x = 0; x < logged_suites; x++)
+							free(tmpdir[x]);
+						free(tmpdir);
 						goto err_out;
+					}
 				}
 			}
 		}
@@ -681,6 +801,11 @@ 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);
 		}
+
+		/* Cleanup logs; on err_out logs are intentionally kept for debugging */
+		for (size_t x = 0; x < logged_suites; x++)
+			cleanup_shell_logs(tmpdir[x]);
+		free(tmpdir);
 	}
 err_out:
 	signal(SIGINT, SIG_DFL);
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index 33f1e46267f4..fc5e1b835ecc 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -322,6 +322,7 @@ static struct test_suite *prepare_test_suite(int dir_fd)
 
 	test_info->base_path = strdup_check(dirpath);		/* Absolute path to dir */
 	test_info->has_setup = NO_SETUP;
+	test_info->store_logs = false;
 
 	test_suite->priv = test_info;
 	test_suite->desc = NULL;
@@ -506,6 +507,9 @@ static void append_suites_in_dir(int dir_fd,
 			continue;
 		}
 
+		/* Store logs for testsuites in sub-directories */
+		((struct shell_test_info *)(test_suite->priv))->store_logs = true;
+		num_logged_suites++;
 		if (is_test_script(fd, SHELL_SETUP)) {	/* Check for setup existence */
 			char *desc = shell_test__description(fd, SHELL_SETUP);
 
diff --git a/tools/perf/tests/tests-scripts.h b/tools/perf/tests/tests-scripts.h
index 013031239883..1dd91f1444e3 100644
--- a/tools/perf/tests/tests-scripts.h
+++ b/tools/perf/tests/tests-scripts.h
@@ -16,8 +16,11 @@ enum shell_setup {
 struct shell_test_info {
 	const char *base_path;
 	enum shell_setup has_setup;
+	bool store_logs;
 };
 
 struct test_suite **create_script_test_suites(void);
 
+extern size_t num_logged_suites;
+
 #endif /* TESTS_SCRIPTS_H */
-- 
2.52.0

[PATCH v7 5/7] perf test: Format log directories for shell tests

[Jakub Brnak]

The name of the log directory can be taken from the test suite
description, which possibly could contain whitespace characters. This
can cause further issues if the name is not quoted correctly.

Replace the whitespace characters with an underscore to prevent the
possible issues caused by the name splitting.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/builtin-test.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 306ecfe5e22f..6cdff3687a82 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -342,16 +342,28 @@ static bool create_logs(struct test_suite *t, int pass)
 	return false;
 }
 
+static const char PERF_LOG_PREFIX[] = "/tmp/perf_test_";
+static const char PERF_LOG_SUFFIX[] = ".XXXXXX";
+
 static char *setup_shell_logs(const char *name)
 {
 	char template[PATH_MAX];
 	char *temp_dir;
+	size_t i, len;
 
-	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", name) >= PATH_MAX) {
+	if (snprintf(template, PATH_MAX, "%s%s%s", PERF_LOG_PREFIX, name,
+		     PERF_LOG_SUFFIX) >= PATH_MAX) {
 		pr_err("Failed to create log dir template");
 		return NULL; /* Skip the testsuite */
 	}
 
+	/* Replace non-alphanumeric characters with _ in the name part */
+	len = strlen(template);
+	for (i = sizeof(PERF_LOG_PREFIX) - 1; i < len - (sizeof(PERF_LOG_SUFFIX) - 1); i++) {
+		if (!isalnum((unsigned char)template[i]))
+			template[i] = '_';
+	}
+
 	temp_dir = mkdtemp(template);
 	if (temp_dir) {
 		setenv("PERFSUITE_RUN_DIR", temp_dir, 1);
-- 
2.52.0

[PATCH v7 6/7] perf test: Remove perftool drivers

[Jakub Brnak]

The perf now provides all of the features required for running the
perftool test cases, such as creating log directories, running
setup scripts and the tests are structured by the base_ directories.

Remove the drivers as they are no longer necessary together with
the condition of skipping the base_ directories and run the
test cases by the default perf test structure.

Signed-off-by: Michael Petlan <mpetlan@redhat.com>
Co-developed-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 .../tests/shell/perftool-testsuite_probe.sh   | 24 -------------------
 .../tests/shell/perftool-testsuite_report.sh  | 23 ------------------
 tools/perf/tests/tests-scripts.c              |  3 ---
 3 files changed, 50 deletions(-)
 delete mode 100755 tools/perf/tests/shell/perftool-testsuite_probe.sh
 delete mode 100755 tools/perf/tests/shell/perftool-testsuite_report.sh

diff --git a/tools/perf/tests/shell/perftool-testsuite_probe.sh b/tools/perf/tests/shell/perftool-testsuite_probe.sh
deleted file mode 100755
index 3863df16c19b..000000000000
--- a/tools/perf/tests/shell/perftool-testsuite_probe.sh
+++ /dev/null
@@ -1,24 +0,0 @@
-#!/bin/bash
-# perftool-testsuite_probe (exclusive)
-# SPDX-License-Identifier: GPL-2.0
-
-[ "$(id -u)" = 0 ] || exit 2
-test -d "$(dirname "$0")/base_probe" || exit 2
-cd "$(dirname "$0")/base_probe" || exit 2
-status=0
-
-PERFSUITE_RUN_DIR=$(mktemp -d /tmp/"$(basename "$0" .sh)".XXX)
-export PERFSUITE_RUN_DIR
-
-for testcase in setup.sh test_*; do                  # skip setup.sh if not present or not executable
-     test -x "$testcase" || continue
-     ./"$testcase"
-     (( status += $? ))
-done
-
-if ! [ "$PERFTEST_KEEP_LOGS" = "y" ]; then
-	rm -rf "$PERFSUITE_RUN_DIR"
-fi
-
-test $status -ne 0 && exit 1
-exit 0
diff --git a/tools/perf/tests/shell/perftool-testsuite_report.sh b/tools/perf/tests/shell/perftool-testsuite_report.sh
deleted file mode 100755
index a8cf75b4e77e..000000000000
--- a/tools/perf/tests/shell/perftool-testsuite_report.sh
+++ /dev/null
@@ -1,23 +0,0 @@
-#!/bin/bash
-# perftool-testsuite_report (exclusive)
-# SPDX-License-Identifier: GPL-2.0
-
-test -d "$(dirname "$0")/base_report" || exit 2
-cd "$(dirname "$0")/base_report" || exit 2
-status=0
-
-PERFSUITE_RUN_DIR=$(mktemp -d /tmp/"$(basename "$0" .sh)".XXX)
-export PERFSUITE_RUN_DIR
-
-for testcase in setup.sh test_*; do                  # skip setup.sh if not present or not executable
-     test -x "$testcase" || continue
-     ./"$testcase"
-     (( status += $? ))
-done
-
-if ! [ "$PERFTEST_KEEP_LOGS" = "y" ]; then
-	rm -rf "$PERFSUITE_RUN_DIR"
-fi
-
-test $status -ne 0 && exit 1
-exit 0
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index fc5e1b835ecc..a83e30ee4a09 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -487,9 +487,6 @@ static void append_suites_in_dir(int dir_fd,
 		if (!is_directory_fd(dir_fd, ent))
 			continue;
 
-		if (strncmp(ent->d_name, "base_", 5) == 0)
-			continue; /* Skip scripts that have a separate driver. */
-
 		/* Scan subdir for test cases */
 		fd = openat(dir_fd, ent->d_name, O_PATH);
 		if (fd < 0)
-- 
2.52.0

[PATCH v7 7/7] perf test: Fix relative path for 'stderr-whitelist.txt'

[Jakub Brnak]

The test scripts for 'perf report' profiles were failing to locate
the 'stderr-whitelist.txt' file.

The path was specified simply as "stderr-whitelist.txt", which relies on
the current working directory when the script runs.

This patch updates the path to use the relative directory variable
'$DIR_PATH' (which points to the current directory of the test script),
ensuring the correct file is found regardless of the current working
directory.

Signed-off-by: Jakub Brnak <jbrnak@redhat.com>
---
 tools/perf/tests/shell/base_report/test_basic.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/base_report/test_basic.sh b/tools/perf/tests/shell/base_report/test_basic.sh
index 0dfe7e5fd1ca..b42726655d20 100755
--- a/tools/perf/tests/shell/base_report/test_basic.sh
+++ b/tools/perf/tests/shell/base_report/test_basic.sh
@@ -240,7 +240,7 @@ PERF_EXIT_CODE=$?
 	"# Overhead   Latency  Command" < $LOGS_DIR/latency_default.log
 CHECK_EXIT_CODE=$?
 "$DIR_PATH/../common/check_errors_whitelisted.pl" \
-	"stderr-whitelist.txt" < $LOGS_DIR/latency_default.err
+	"$DIR_PATH/stderr-whitelist.txt" < $LOGS_DIR/latency_default.err
 (( CHECK_EXIT_CODE += $? ))
 
 print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "default report for latency profile"
@@ -255,7 +255,7 @@ PERF_EXIT_CODE=$?
 	"#  Latency  Overhead  Command" < $LOGS_DIR/latency_latency.log
 CHECK_EXIT_CODE=$?
 "$DIR_PATH/../common/check_errors_whitelisted.pl" \
-	"stderr-whitelist.txt" < $LOGS_DIR/latency_latency.err
+	"$DIR_PATH/stderr-whitelist.txt" < $LOGS_DIR/latency_latency.err
 (( CHECK_EXIT_CODE += $? ))
 
 print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "latency report for latency profile"
@@ -271,7 +271,7 @@ PERF_EXIT_CODE=$?
 	< $LOGS_DIR/parallelism_hierarchy.log
 CHECK_EXIT_CODE=$?
 "$DIR_PATH/../common/check_errors_whitelisted.pl" \
-	"stderr-whitelist.txt" < $LOGS_DIR/parallelism_hierarchy.err
+	"$DIR_PATH/stderr-whitelist.txt" < $LOGS_DIR/parallelism_hierarchy.err
 (( CHECK_EXIT_CODE += $? ))
 
 print_results $PERF_EXIT_CODE $CHECK_EXIT_CODE "parallelism histogram"
-- 
2.52.0

Re: [PATCH v7 1/7] perf tests: Create a structure for shell tests

[sashiko-bot]

Sashiko AI review found 3 potential issue(s):
- [High] Stack buffer overflow when constructing file paths with readlink().
- [High] Unsanitized file paths are passed to system(), leading to test failures and arbitrary command execution.
- [Medium] Inconsistent symlink traversal in `is_directory_fd` can cause infinite recursion.
--

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

perf tests: Create a structure for shell tests

This commit implements a new structure for shell test suites, grouping them
by subdirectories within perf/tests/shell instead of placing them all into a
single list.

> diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
> index f18c4cd337c8f..680dfe650f146 100644
> --- a/tools/perf/tests/tests-scripts.c
> +++ b/tools/perf/tests/tests-scripts.c
> @@ -151,14 +200,64 @@ static char *strdup_check(const char *str)
>  	return newstr;
>  }
>  
> -static int shell_test__run(struct test_suite *test, int subtest __maybe_unused)
> +/* Check if dirent entry is a directory using file descriptor, handling DT_UNKNOWN case */
> +static bool is_directory_fd(int dir_fd, struct dirent *ent)
>  {
> -	const char *file = test->priv;
> +	struct stat st;
> +
> +	if (ent->d_type == DT_DIR)
> +		return true;
> +
> +	if (ent->d_type != DT_UNKNOWN)
> +		return false;
> +
> +	/* Need to stat the file to determine if it's a directory */
> +	if (fstatat(dir_fd, ent->d_name, &st, 0) != 0)
> +		return false;

Could this cause inconsistent symlink traversal depending on the filesystem?
If the filesystem returns DT_UNKNOWN, fstatat() without the AT_SYMLINK_NOFOLLOW
flag will follow symlinks and report them as directories. This differs from the
fast path which correctly skips symlinks because their d_type is DT_LNK.
Could this lead to infinite recursion if a symlink loops back to a parent
directory?

> +
> +	return S_ISDIR(st.st_mode);
> +}

[ ... ]

> +static int shell_test__run(struct test_suite *test, int subtest)
> +{
> +	const char *file;
>  	int err;
>  	char *cmd = NULL;
>  
> +	/* Get absolute file path */
> +	if (subtest >= 0)
> +		file = test->test_cases[subtest].name;
> +	else	/* Single test case */
> +		file = test->test_cases[0].name;
> +
>  	if (asprintf(&cmd, "%s%s", file, verbose ? " -v" : "") < 0)
>  		return TEST_FAIL;
> +
>  	err = system(cmd);

Since file contains the absolute path to the test script, what happens if the
kernel source tree is located in a directory path containing spaces or shell
metacharacters? Will system() fail to execute the path properly?

[ ... ]

> @@ -167,65 +266,117 @@ static int shell_test__run(struct test_suite *test, int subtest __maybe_unused)
>  	return WEXITSTATUS(err) == 2 ? TEST_SKIP : TEST_FAIL;
>  }
>  
> -static void append_script(int dir_fd, const char *name, char *desc,
> -			  struct test_suite ***result,
> -			  size_t *result_sz)
> +static struct test_suite *prepare_test_suite(int dir_fd)
>  {
> -	char filename[PATH_MAX], link[128];
> -	struct test_suite *test_suite, **result_tmp;
> -	struct test_case *tests;
> +	char dirpath[PATH_MAX], link[128];
>  	ssize_t len;
> -	char *exclusive;
> +	struct test_suite *test_suite = NULL;
> +	struct shell_test_info *test_info;
>  
> +	/* Get dir absolute path */
>  	snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
> -	len = readlink(link, filename, sizeof(filename));
> +	len = readlink(link, dirpath, sizeof(dirpath));
>  	if (len < 0) {
>  		pr_err("Failed to readlink %s", link);
> -		return;
> +		return NULL;
>  	}
> -	filename[len++] = '/';
> -	strcpy(&filename[len], name);
> +	dirpath[len++] = '/';
> +	dirpath[len] = '\0';

If readlink() fills the entire dirpath array, returning a length of PATH_MAX,
won't these assignments write out of bounds and corrupt the stack?

[ ... ]

> +static void append_script_to_suite(int dir_fd, const char *name, char *desc,
> +					struct test_suite *test_suite, size_t *tc_count)
> +{
> +	char file_name[PATH_MAX], link[128];
> +	struct test_case *tests;
> +	ssize_t len;
> +	char *exclusive;

[ ... ]

> +	/* Get path to the test script */
> +	snprintf(link, sizeof(link), "/proc/%d/fd/%d", getpid(), dir_fd);
> +	len = readlink(link, file_name, sizeof(file_name));
> +	if (len < 0) {
> +		pr_err("Failed to readlink %s", link);
> +		free(desc);
> +		return;
> +	}
> +	file_name[len++] = '/';
> +	strcpy(&file_name[len], name);

Similarly here, if readlink() returns PATH_MAX, file_name[len++] will write past
the end of the array. Also, won't strcpy() overflow the buffer if there isn't
enough space left in file_name to hold the script name?

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

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

[sashiko-bot]

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

Re: [PATCH v7 4/7] perf test: Introduce storing logs for shell tests

[sashiko-bot]

Sashiko AI review found 6 potential issue(s):
- [High] Returning 1 from the `nftw` callback to enforce a depth limit incorrectly aborts the entire directory traversal, resulting in permanent temporary file leaks.
- [High] The directory filtering logic in `delete_file` fails to protect the contents of unknown directories and causes parent directories to be leaked.
- [High] The size calculation for the log directory array `tmpdir` is vulnerable to integer overflow on 32-bit systems, leading to a heap buffer overflow.
- [Medium] The `PERFSUITE_RUN_DIR` environment variable is leaked to subsequent test suites that do not request logs.
- [Medium] Test suites are executed even when log directory creation fails, causing them to use a stale environment.
- [Medium] Using unsanitized test suite descriptions in the `mkdtemp` template causes predictable directory creation failures or shell variable escaping issues.
--

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

perf test: Introduce storing logs for shell tests

Create temporary directories for storing log files for shell tests
that could help while debugging. If the variable PERFTEST_KEEP_LOGS
is set keep the logs, else delete them.

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 76fc37b440be4..306ecfe5e22ff 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c

[ ... ]

> @@ -288,6 +292,91 @@ static bool test_exclusive(const struct test_suite *t, int test_case)
>  	return t->test_cases[test_case].exclusive;
>  }
>  
> +static int delete_file(const char *fpath, const struct stat *sb __maybe_unused,
> +		       int typeflag, struct FTW *ftwbuf)
> +{
> +	/* Stop traversal if going too deep */
> +	if (ftwbuf->level > 5) {
> +		pr_err("Tree traversal reached level %d, stopping.", ftwbuf->level);
> +		return 1;
> +	}

Does returning 1 from the nftw callback incorrectly abort the entire
directory traversal? Per POSIX specifications, returning a non-zero
value stops nftw completely. This could abandon all remaining unvisited
files and directories within the test's log directory and potentially
lead to /tmp exhaustion.

> +
> +	/* Remove only expected directories */
> +	if (typeflag == FTW_D || typeflag == FTW_DP) {
> +		const char *dirname = fpath + ftwbuf->base;
> +
> +		if (strcmp(dirname, "logs") && strcmp(dirname, "examples") &&
> +		    strcmp(dirname, "header_tar") && strncmp(dirname, "perf_", 5)) {
> +			pr_err("Unknown directory %s", dirname);
> +			return 0;
> +		}
> +	}
> +
> +	/* Attempt to remove the file, continue on failure */
> +	if (remove(fpath))
> +		pr_err("Failed to remove file: %s", fpath);
> +
> +	return 0;
> +}

Because nftw is called with FTW_DEPTH (post-order traversal), files inside
the directory are visited first. For regular files (FTW_F), this directory
name check is bypassed, and remove() unconditionally deletes them.

Are all files within the protected directory deleted before the directory
itself is evaluated? 

Also, because the now-empty unknown directory is not deleted, would the
parent directory's subsequent remove() fail with ENOTEMPTY, leaking the
directory structure?

[ ... ]

> +static char *setup_shell_logs(const char *name)
> +{
> +	char template[PATH_MAX];
> +	char *temp_dir;
> +
> +	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", name) >= PATH_MAX) {
> +		pr_err("Failed to create log dir template");
> +		return NULL; /* Skip the testsuite */
> +	}
> +
> +	temp_dir = mkdtemp(template);

Can the test suite description string contain slashes or spaces? Slashes
would cause mkdtemp to fail with ENOENT because intermediate directories do
not exist. Spaces could cause issues when the unquoted PERFSUITE_RUN_DIR
is used in shell scripts.

I see this issue is addressed later in the series in commit
deaef0503b790e27361344881ded63d280631c12.

> +	if (temp_dir) {
> +		setenv("PERFSUITE_RUN_DIR", temp_dir, 1);
> +		return strdup(temp_dir);
> +	}
> +
> +	pr_err("Failed to create the temporary directory");
> +
> +	return NULL; /* Skip the testsuite */
> +}
> +
> +static void cleanup_shell_logs(char *dirname)
> +{
> +	char *keep_logs = getenv("PERFTEST_KEEP_LOGS");
> +
> +	/* Check if logs should be kept or do cleanup */
> +	if (dirname) {
> +		if (!keep_logs || strcmp(keep_logs, "y") != 0)
> +			nftw(dirname, delete_file, 8, FTW_DEPTH | FTW_PHYS);
> +		free(dirname);
> +	}
> +
> +	unsetenv("PERFSUITE_RUN_DIR");
> +}

Is the PERFSUITE_RUN_DIR environment variable leaked to subsequent test
suites that do not request logs? The unsetenv() is only called during
cleanup_shell_logs(). If a test does not request logs, could it inherit
the value pointing to the previous test's directory?

[ ... ]

> @@ -627,9 +716,19 @@ static int __cmd_test(struct test_suite **suites, int argc, const char *argv[],
>  	 * runs the exclusive tests sequentially. In other modes all tests are
>  	 * run in pass 1.
>  	 */
> +
>  	for (int pass = 1; pass <= 2; pass++) {
>  		int child_test_num = 0;
>  		int curr_suite = 0;
> +		size_t tmpdir_sz = num_logged_suites * runs_per_test;
> +		char **tmpdir = tmpdir_sz ? calloc(tmpdir_sz, sizeof(*tmpdir)) : NULL;

Is there an integer overflow risk on 32-bit systems here? The runs_per_test
variable is controlled via a command-line argument. Providing a large value
could cause the multiplication to wrap around, resulting in calloc allocating
a small buffer, which may lead to an out-of-bounds heap write during the
loop.

[ ... ]

>  			for (unsigned int run = 0; run < runs_per_test; run++) {
> +				/* Setup temporary log directories for shell test suites */
> +				if (create_logs(*t, pass)) {
> +					struct shell_test_info *info = (*t)->priv;
> +
> +					tmpdir[logged_suites] = setup_shell_logs((*t)->desc);
> +					/* Couldn't create log dir, skip test suite */
> +					if (tmpdir[logged_suites] == NULL) {
> +						if (info->has_setup != NO_SETUP)
> +							info->has_setup = FAILED_SETUP;
> +					} else {
> +						logged_suites++;
> +						if (info->has_setup == PASSED_SETUP ||
> +						    info->has_setup == FAILED_SETUP)
> +							info->has_setup = RUN_SETUP;
> +					}
> +				}
> +
>  				test_suite__for_each_test_case(*t, curr_test_case) {

The comment says to skip the test suite, but there is no continue statement
here. Are the test cases still executed without the intended logging setup
if setup_shell_logs() returns NULL?

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

Re: [PATCH v7 5/7] perf test: Format log directories for shell tests

[sashiko-bot]

Sashiko AI review found 2 potential issue(s):
- [Medium] Temporary directory resource leak when strdup() fails.
- [Low] Implementation of character replacement is overly aggressive and misaligned with the commit intent.
--

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

perf test: Format log directories for shell tests

This commit creates a temporary directory for shell tests using a template based
on the test suite name. It replaces non-alphanumeric characters with underscores
to avoid issues with whitespace or special characters when logging.

>     Replace the whitespace characters with an underscore to prevent the
>     possible issues caused by the name splitting.

This isn't a bug, but the commit message mentions replacing whitespace
characters, while the implementation replaces all non-alphanumeric characters.
Could the commit message be updated to match the code?

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 306ecfe5e22ff..6cdff3687a828 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
[ ... ]
> @@ -342,16 +342,28 @@ static bool create_logs(struct test_suite *t, int pass)
>  	return false;
>  }
>  
> +static const char PERF_LOG_PREFIX[] = "/tmp/perf_test_";
> +static const char PERF_LOG_SUFFIX[] = ".XXXXXX";
> +
>  static char *setup_shell_logs(const char *name)
>  {
>  	char template[PATH_MAX];
>  	char *temp_dir;
> +	size_t i, len;
>  
> -	if (snprintf(template, PATH_MAX, "/tmp/perf_test_%s.XXXXXX", name) >= PATH_MAX) {
> +	if (snprintf(template, PATH_MAX, "%s%s%s", PERF_LOG_PREFIX, name,
> +		     PERF_LOG_SUFFIX) >= PATH_MAX) {
>  		pr_err("Failed to create log dir template");
>  		return NULL; /* Skip the testsuite */
>  	}
>  
> +	/* Replace non-alphanumeric characters with _ in the name part */
> +	len = strlen(template);
> +	for (i = sizeof(PERF_LOG_PREFIX) - 1; i < len - (sizeof(PERF_LOG_SUFFIX) - 1); i++) {
> +		if (!isalnum((unsigned char)template[i]))
> +			template[i] = '_';
> +	}
> +
>  	temp_dir = mkdtemp(template);
>  	if (temp_dir) {
>  		setenv("PERFSUITE_RUN_DIR", temp_dir, 1);

If strdup(temp_dir) fails right after this and returns NULL, do we leave the
newly created temporary directory on the filesystem? 

It looks like mkdtemp() creates the directory, but there is no cleanup if the
memory allocation fails. Additionally, should we unset the PERFSUITE_RUN_DIR
environment variable on this error path?

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

Re: [PATCH v7 6/7] perf test: Remove perftool drivers

[sashiko-bot]

Sashiko AI review found 1 potential issue(s):
- [Critical] Out-of-bounds stack write in `append_script_to_suite` and `prepare_test_suite` when reading directory paths via `/proc/self/fd/`.
--

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

perf test: Remove perftool drivers

The perf now provides all of the features required for running the
perftool test cases, such as creating log directories, running
setup scripts and the tests are structured by the base_ directories.

[ ... ]

> diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
> index fc5e1b835ecc3..a83e30ee4a099 100644
> --- a/tools/perf/tests/tests-scripts.c
> +++ b/tools/perf/tests/tests-scripts.c
> @@ -487,9 +487,6 @@ static void append_suites_in_dir(int dir_fd,
>  		if (!is_directory_fd(dir_fd, ent))
>  			continue;
>  
> -		if (strncmp(ent->d_name, "base_", 5) == 0)
> -			continue; /* Skip scripts that have a separate driver. */
> -

Since this patch exposes base_ directories to the parsing logic, can this
lead to an out-of-bounds stack write in append_script_to_suite() when reading
directory paths via /proc/self/fd/?

Looking at append_script_to_suite(), readlink() reads the directory path into
a stack buffer file_name[PATH_MAX]:

append_script_to_suite() {
	...
	char file_name[PATH_MAX], link[128];
	...
	len = readlink(link, file_name, sizeof(file_name));
	if (len < 0) {
		...
	}
	file_name[len++] = '/';
	strcpy(&file_name[len], name);
	...
}

readlink() does not null-terminate and returns up to the buffer size.

If the directory's absolute path length is PATH_MAX or greater, readlink()
returns PATH_MAX.

The subsequent code file_name[len++] = '/' then writes to file_name[PATH_MAX],
which is past the end of the array.

Does the following strcpy() overflow file_name[], causing a regression with
a stack buffer overflow?

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