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

[Gabriele Monaco <gmonaco@redhat.com>]

On Thu, 2026-08-20 at 16:58 +0000, sashiko-bot@kernel.org wrote:
> > +  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.

I'm wondering if this is a broad example of root can shoot themselves in the
foot.
This is a selftest run by a root user, I believe they are responsible to
validate what's running isn't malicious.

I cannot picture a case where a path is commonly world writable here. I would
keep things simple and perhaps walk up at a maximum of 4-5 levels
(tools/testing/selftests/verification/test.d), to kinda make sure we never go
out of the repository.

You cannot really rely on git (may run from a tarball) or other static methods,
but I believe 4-5 levels up will cover us just fine.

In my opinion all this stat/dirperms thing is rarely useful, you could just
write the concern in a *brief* comment, saying we trust 4-5 level ups are still
in the repo and we trust the repo, just to make AIs happy.

What do you think?

Gabriele

> 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
> >  }