Re: [PATCH v5 9/9] selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Local Privilege Escalation via untrusted directory traversal when implicitly sourcing test.d/functions.
- [Medium] Infinite loop in directory traversal when testing top-level directories due to POSIX 'dirname' behavior with double slashes.
--

--- Patch [9]: [PATCH v5 9/9] selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed ---
commit 982c65a7f93c6d254f86b734c29fd401b0e3707a
Author: Wen Yang <wen.yang@linux.dev>

selftests/ftrace: Walk up to find test.d/functions when a subdirectory is passed

When a test directory that does not itself contain test.d/functions is
passed to ftracetest (e.g. verification/test.d/tlob/), ftracetest fell
back to its own functions file and lost the rv-specific check_requires
handling for ':monitor' and ':reactor' requirements.

> diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest
> index 0a56bf209f6c0..3ba929820dd2c 100755
> --- a/tools/testing/selftests/ftrace/ftracetest
> +++ b/tools/testing/selftests/ftrace/ftracetest
> @@ -159,9 +159,20 @@ 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.
> +    dir=$OPT_TEST_DIR
> +    while [ "$dir" != "/" ]; do

[Severity: Medium]
Can this loop run indefinitely if a user passes a directory directly under
the root, like /tmp?

If the path is formatted as //tmp, GNU dirname preserves double leading
slashes because it treats them as POSIX UNC paths. If dir starts as //tmp,
dirname "$dir" evaluates to //, and the loop condition [ "$dir" != "/" ]
will evaluate to true infinitely, causing the test script to hang.

> +      if [ -f "$dir/test.d/functions" ]; then
> +        TOP_DIR=$dir
> +        TEST_DIR=$TOP_DIR/test.d
> +        break
> +      fi

[Severity: High]
Could this directory traversal lead to arbitrary code execution if the path
walks back through a world-writable directory?

If ftracetest is run as root on a trusted directory inside a path like
/tmp/legitimate_tests/, the loop will traverse up to /tmp and search for
/tmp/test.d/functions.

An unprivileged attacker could preemptively plant a malicious script at
that location. The script would then be found, and since the ftracetest
script later sources this file, it would execute the attacker's code with
root privileges.

> +      dir=$(dirname "$dir")
> +    done
> +  fi
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787161646.git.wen.yang@linux.dev?part=9