Re: [PATCH bpf-next v1 06/11] selftests/bpf: exercise the path ancestor iterator

[Justin Suess <utilityemal77@gmail.com>]

On Tue, Oct 13, 2026 at 12:47:55PM -0400, Justin Suess wrote:
> Walk the ancestry of a mkdir's parent directory from a sleepable LSM
> program and check that the iteration reaches the root: the temporary
> directory, its parent and / are visited, no position comes back
> flagged, and the flags read back after the loop show it ended at the
> root rather than on a failed allocation.
> 
> Hold the first position across the rest of the walk and resolve its
> pathname only once the loop is done and the iterator destroyed: the
> acquired reference, not the walk's, is then all that keeps the
> position alive.
> 
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/path_ancestors.c | 57 ++++++++++++++
>  .../selftests/bpf/progs/path_ancestors.c      | 74 +++++++++++++++++++
>  2 files changed, 131 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/path_ancestors.c
>  create mode 100644 tools/testing/selftests/bpf/progs/path_ancestors.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/path_ancestors.c b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> new file mode 100644
> index 000000000000..d1bd26bce10b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/path_ancestors.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Justin Suess */
> +
> +#include <sys/stat.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <test_progs.h>
> +#include "path_ancestors.skel.h"
> +
> +void test_path_ancestors(void)
> +{
> +	char base[] = "/tmp/path_ancestors_XXXXXX";
> +	struct path_ancestors *skel = NULL;
> +	char suba[280], subb[280];
> +
> +	if (!ASSERT_OK_PTR(mkdtemp(base), "mkdtemp"))
> +		return;
> +	snprintf(suba, sizeof(suba), "%s/a", base);
> +	snprintf(subb, sizeof(subb), "%s/a/b", base);
> +	if (!ASSERT_OK(mkdir(suba, 0755), "mkdir_a"))
> +		goto out_rm;
> +
> +	skel = path_ancestors__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		goto out_rm;
> +	skel->bss->monitored_pid = getpid();
> +	if (!ASSERT_OK(path_ancestors__attach(skel), "attach"))
> +		goto out;
> +
> +	/* mkdir b: the hook sees dir == suba, whose ancestry is walked. */
> +	if (!ASSERT_OK(mkdir(subb, 0755), "mkdir_b"))
> +		goto out;
> +
> +	/* suba, base, /tmp, / at least. */
> +	ASSERT_GE(skel->bss->ref_count, 4, "ref_count");
> +	ASSERT_EQ(skel->bss->ref_flags, 0, "ref_flags");
> +	/*
> +	 * The referenced loop ended at the root, not on an allocation
> +	 * failure.
> +	 */
> +	ASSERT_EQ(skel->bss->end_flags, 0, "end_flags");
> +
> +	/* The acquired second position, used after its step was taken. */
> +	ASSERT_STREQ(skel->bss->second_path, base, "second_path");
> +	ASSERT_EQ(skel->bss->second_len, strlen(base) + 1, "second_len");
> +
> +	/* The first position, used only after the walk itself was gone. */
> +	ASSERT_STREQ(skel->bss->first_path, suba, "first_path");
> +	ASSERT_EQ(skel->bss->first_len, strlen(suba) + 1, "first_len");
> +
> +out:
> +	path_ancestors__destroy(skel);
> +out_rm:
> +	rmdir(subb);
> +	rmdir(suba);
> +	rmdir(base);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
> new file mode 100644
> index 000000000000..6287f5099a18
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/path_ancestors.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Justin Suess */
> +
> +#include "vmlinux.h"
> +#include <errno.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "bpf_kfuncs.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +__u32 monitored_pid;
> +
> +int ref_count;		/* positions seen by the pure referenced walk */
> +int ref_flags;		/* pos flags seen by the referenced walk */
> +int end_flags;		/* flags after a referenced loop ended: 0 unless NOMEM */
> +int second_len;		/* d_path length of the walk's second position */
> +int first_len;		/* d_path length of the first position, after the walk */
> +char second_path[256];
> +char first_path[256];
> +
> +static bool monitored(void)
> +{
> +	return (bpf_get_current_pid_tgid() >> 32) == monitored_pid;
> +}
> +
> +SEC("lsm.s/path_mkdir")
> +int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,
> +	     umode_t mode)
> +{
> +	struct bpf_iter_path_ancestors it;
> +	struct path *pos, *first = NULL;
> +
> +	/*
> +	 * Keep first's init opaque: gcc-bpf otherwise folds it into a
> +	 * preceding loop's exit value, which the verifier cannot track.
> +	 */
> +	barrier_var(first);
> +
cc'ing Eduard Zingerman + Jose Marchesi

This barrier_var line was necessary for this selftest to pass
under GCC BPF.

Othwise it will be rejected by the verifier:

  84: (85) call bpf_iter_path_ancestors_rcu_next#63514
  85: R0=rcu_ptr_path(id=30) ... depth=9
  86: (bf) r6 = r0              ; R6=rcu_ptr_path(id=30)
  88: (55) if r0 != 0x0 goto pc-64
  ...
  101: (85) call bpf_rcu_read_unlock#64876
       ^-- R6 downgraded rcu_ptr_ -> untrusted_ptr_
  ...
  146: (15) if r6 == 0x0 goto pc+10     ; R6=untrusted_ptr_path(id=30)
  147: (bf) r1 = r6
  151: (85) call bpf_path_d_path#64649
  R1 must be referenced or trusted

I think it's trying to optimize out the "struct path* first" and make it
use the r6 register. It tries reusing the R6 register between the two
loops, and doing so causes the program to fail to verify because it doesn't
understand that the path was reinitialized with r6=0 from the previous
exit.

So I had to add the memory barrier to force struct path to be
instantiated outside the register.

There's nothing wrong from a *correctness* perspective with the code
generated by GCC-BPF. But I'm not sure if this is a GCC-BPF bug or a
bug in the SCEV widening series (merge 15b578b1715d). It depends on
the contract BPF bytecode is supposed to meet.

But clang works without the barrier, it's only GCC-BPF that produces
this issue requiring the barrier.

I can give more details if needed.

Justin
> [...]