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

[bot+bpf-ci at kernel.org]

> selftests/bpf: exercise the lockless path ancestor iterator

TL;DR: Commit message and test comment call d_path on the escalated position
"sleepable work", but bpf_path_d_path() doesn't look KF_SLEEPABLE; it likely
needs a trusted pointer. Wording only, no code bug suggested.

>     selftests/bpf: exercise the lockless path ancestor iterator
>
>     Walk the same ancestry four ways and require the position counts to
>     agree: lockless from a non-sleepable program, where the RCU critical
>     section is implicit; lockless under an explicit bpf_rcu_read_lock();
>     referenced; and the hybrid that walks lockless to the second position,
>     hands it over to a referenced iteration, and resumes there - the
>     escalated position therefore being walked twice, once per mode.
>
>     The sleepable work the escalation exists for (d_path on the escalated
>     position) runs on the resumed iteration's first position, after
>     bpf_rcu_read_unlock(), which is the only place a sleepable kfunc can
>     run at all.
>
>     Signed-off-by: Justin Suess <utilityemal77 at gmail.com>

The commit message calls d_path on the escalated position "the sleepable
work the escalation exists for".  Is bpf_path_d_path() a sleepable kfunc?

It is registered without KF_SLEEPABLE in fs/bpf_fs_kfuncs.c, unlike
bpf_get_dentry_xattr() right next to it:

    BTF_ID_FLAGS(func, bpf_path_d_path)
    BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE)

The reason the test needs the escalated position for d_path looks like trust
rather than sleepability.  bpf_iter_path_ancestors_rcu_new() is
KF_RCU_PROTECTED, so the verifier types the pointer returned by
bpf_iter_path_ancestors_rcu_next() as PTR_TO_BTF_ID | MEM_RCU.
bpf_path_d_path() is not KF_RCU, so it needs a trusted argument and would
reject that pointer.  The previous patch's changelog describes the same
constraint:

> nothing read out of the position may be passed
> to a kfunc demanding a trusted argument.

The comment in the test program has the same framing:

> diff --git a/tools/testing/selftests/bpf/progs/path_ancestors.c b/tools/testing/selftests/bpf/progs/path_ancestors.c
> index 6287f5099a18..3ebf86b23199 100644
> --- a/tools/testing/selftests/bpf/progs/path_ancestors.c
> +++ b/tools/testing/selftests/bpf/progs/path_ancestors.c

[ ... ]

> @@ -70,5 +111,36 @@ int BPF_PROG(walk_modes, const struct path *dir, struct dentry *dentry,

[ ... ]

> +	/*
> +	 * Out of the RCU critical section.  The resumed iteration's first
> +	 * position is the escalated one, kept alive by the reference the
> +	 * iteration hands out, so sleepable work can run on it.
> +	 */
> +	while ((pos = bpf_iter_path_ancestors_next(&it))) {
> +		hybrid_count++;
> +		if (hybrid_count == 3)
> +			escalated_len = bpf_path_d_path(pos, escalated_path,
> +							sizeof(escalated_path));

Beyond the iterator's own bpf_iter_path_ancestors_next() and
bpf_path_put(), does the test run any sleepable work on the escalated
position?

Could the commit message (and this comment) say that d_path needs a
trusted/referenced position, or could the test call a KF_SLEEPABLE kfunc
such as bpf_get_dentry_xattr() on the escalated position so that the
description matches what is checked?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38437592296