Re: [PATCH v6 9/9] selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed
From: sashiko-bot@kernel.org
Date: 2026-08-20 16:58:20
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The directory traversal logic fails to check directory ownership and fails open on `stat` error, allowing arbitrary code execution as root via an attacker-controlled ancestor directory. --
--- Patch [9]: [PATCH v6 9/9] selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed ---commit 71305ea3fc4b8a7294d098d4bcdd8624fd21ca70 Author: Wen Yang [off-list ref] selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed This commit modifies ftracetest to walk up the directory tree from a passed test directory until it finds a test.d/functions file. This allows monitor subdirectories to be used directly as the test root without needing dummy functions shims.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index 0a56bf209f6c0..8f9d9291bf4ce 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest
[ ... ]
quoted hunk ↗ jump to hunk
@@ -159,9 +159,29 @@ parse_opts() { # opts if [ -n "$OPT_TEST_CASES" ]; then TEST_CASES=$OPT_TEST_CASES fi - if [ -n "$OPT_TEST_DIR" -a -f "$OPT_TEST_DIR"/test.d/functions ]; then - TOP_DIR=$OPT_TEST_DIR - TEST_DIR=$TOP_DIR/test.d + if [ -n "$OPT_TEST_DIR" ]; then + # Walk up from OPT_TEST_DIR to find the nearest ancestor that contains + # test.d/functions. This allows a monitor subdirectory (e.g. + # verification/test.d/tlob/) to be passed directly without placing a + # dummy functions shim in each new subdirectory. + # Security: only walk within directories owned by root or the current user + # to prevent sourcing a functions file planted in a world-writable ancestor. + dir=$(realpath "$OPT_TEST_DIR" 2>/dev/null || echo "$OPT_TEST_DIR") + while [ "$dir" != "/" ] && [ "$dir" != "//" ]; do + if [ -f "$dir/test.d/functions" ]; then + # Verify the directory is not world-writable to prevent LPE. + dirperms=$(stat -c "%a" "$dir" 2>/dev/null || stat -f "%Lp" "$dir" 2>/dev/null) + case "$dirperms" in + *2|*3|*6|*7) ;; # world-writable: skip this candidate + *) + TOP_DIR=$dir
[Severity: High] Does this logic actually check directory ownership as mentioned in the comments? It appears the code only checks if the directory is world-writable, but omits validating that it is owned by root or the current user. An attacker could potentially create a directory with standard 0755 permissions, plant a malicious test.d/functions script, and if a privileged user or automated test harness runs ftracetest against a subdirectory, the script would traverse up and source the payload. Additionally, if the stat command fails for any reason, dirperms will be empty. Would an empty string match the *) fallback case and cause the script to silently fail open, accepting even a world-writable directory?
+ TEST_DIR=$TOP_DIR/test.d + break + ;; + esac + fi + dir=$(dirname "$dir") + done fi }
-- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787243842.git.wen.yang@linux.dev?part=9