[RFC PATCH bpf-next 04/12] bpf: mark struct path trusted

[bot+bpf-ci at kernel.org]

> bpf: mark struct path trusted

TL;DR: Marking struct path trusted may let a MEM_RCU path's dentry (from the
later RCU ancestor iterator) be typed PTR_TRUSTED, so sleepable kfuncs could
possibly use a freed dentry after RCU unlock.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -6308,6 +6308,10 @@ BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args) {
>  	struct sched_ext_ops *ops;
>  };
>
> +BTF_TYPE_SAFE_TRUSTED(struct path) {
> +	struct dentry *dentry;
> +};
> +

Can this entry also apply to a struct path pointer that is MEM_RCU rather
than trusted?

check_ptr_to_btf_access() consults the same allow-list for both kinds of
parent:

	} else if (is_trusted_reg(env, reg) || is_rcu_reg(reg)) {
		...
		if (type_is_trusted(env, reg, field_name, btf_id)) {
			flag |= PTR_TRUSTED;

The commit message only justifies the entry for a trusted struct path:
"Walking a trusted struct path yields a trusted dentry - a live path never
carries a NULL dentry".  The comment above the SAFE_TRUSTED group also says
these fields are trusted even outside of an RCU critical section.  That
holds for LSM hook arguments and for paths returned by an acquiring kfunc,
but it does not seem to hold for a MEM_RCU struct path pointer.

Later commits in this series create such a pointer.  Commit 47d25368b3cc
("bpf: add a lockless path ancestor iterator") adds
bpf_iter_path_ancestors_rcu_new() with KF_RCU_PROTECTED, and
bpf_iter_path_ancestors_rcu_next() with KF_ITER_NEXT | KF_RET_NULL and no
KF_ACQUIRE.  check_kfunc_call() types the result of an iter_next on a
MEM_RCU iterator as PTR_TO_BTF_ID | MEM_RCU, and the kfunc returns
&kit->aw.pos, the walk's own borrowed position.  That commit's message says
nothing read out of the position may be passed to a kfunc demanding a
trusted argument, but with this patch the first thing a program does
breaks that rule:

	pos = bpf_iter_path_ancestors_rcu_next(&rit);	/* PTR_TO_BTF_ID | MEM_RCU */
	d = pos->dentry;	/* now PTR_TO_BTF_ID | PTR_TRUSTED */

The register d carries only PTR_TRUSTED, not MEM_RCU.
invalidate_rcu_protected_refs() only downgrades MEM_RCU registers, so d
stays trusted after bpf_rcu_read_unlock().  A sleepable LSM program could
then do:

	bpf_rcu_read_lock();
	bpf_iter_path_ancestors_rcu_new(&rit, path, 0);
	pos = bpf_iter_path_ancestors_rcu_next(&rit);
	d = pos->dentry;
	...
	bpf_iter_path_ancestors_rcu_destroy(&rit);
	bpf_rcu_read_unlock();
	bpf_get_dentry_xattr(d, ...);	/* or bpf_set_dentry_xattr() /
					   bpf_remove_dentry_xattr() */

The verifier would accept this because d is PTR_TRUSTED.  In an rcu-walk
the position holds no reference on the dentry, so the dentry may already
have been killed and freed once the RCU section has ended.  Can the
sleepable kfunc then operate on a freed dentry?

Even inside the critical section, "trusted" promises a non-zero refcount,
which the lockless walk does not provide.

The position is also storage inside the iterator on the BPF stack, and pos
is not invalidated by bpf_iter_path_ancestors_rcu_destroy().  The destroy
releases the iterator's reference id, but the registers returned by
iter_next carry neither that id nor a parent_id.  So pos->dentry can be
re-read after the slot has been overwritten with program-chosen data, and
the result is still typed as a trusted dentry.  Before this patch the same
load gave a non-trusted pointer, which kfuncs reject.

Nothing later in the series restores the invariant: the verifier.c diff for
the rest of the series (7ed5aa71ad77..70352ce0817c) does not touch
type_is_trusted(), check_ptr_to_btf_access() or this path entry.  The
selftests only read pos->dentry on the referenced iterator, so they do not
exercise this case.

Should this entry apply only to trusted/referenced struct path parents, for
example by not consulting the SAFE_TRUSTED list for MEM_RCU parents of this
type?  Alternatively, could the rcu iterator return a position that the
verifier marks PTR_UNTRUSTED, or could the rcu variant return a dentry that
is MEM_RCU-tagged instead of PTR_TRUSTED?

[ ... ]

> @@ -6360,6 +6364,7 @@ static bool type_is_trusted(struct bpf_verifier_env *env,
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_cpu_release_args));
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_attach_args));
>  	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct scx_sub_detach_args));
> +	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct path));
>
>  	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");
>  }


---
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/37395354107