Re: [PATCH bpf-next v5 02/11] bpf: Add helpers to describe the R0:R2 return register pair
Eduard Zingerman <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 2026-08-13 at 13:02 -0700, Yonghong Song wrote:
...
> +static void bpf_compute_subprog_ret_regs(struct bpf_verifier_env *env)
> +{
> + const struct btf *btf = env->prog->aux->btf;
> + const struct btf_type *type;
> + int subprog;
> + u32 size;
>
> - return btf_type_is_void(type);
> + for (subprog = 0; subprog < env->subprog_cnt; subprog++) {
> + type = subprog_ret_type(env, subprog);
> + if (!type || !(btf_type_is_struct(type) || btf_type_is_scalar(type)))
> + continue;
Is the check '!(btf_type_is_struct(type) || btf_type_is_scalar(type))'
needed after v4->v5 migration?
> + if (IS_ERR(btf_resolve_size(btf, type, &size)))
> + continue;
Let's propagate this error instead of ignoring it. Or add WARN_ON_ONCE().
After BTF validation it shouldn't really happen.
> + if (ret_regs_cnt(size) > 1) {
> + subprog_info(env, subprog)->ret_reg_pair = true;
> + /*
> + * The R0:R2 return convention is only implemented in
> + * the JIT: the interpreter propagates BPF_R0 alone out
> + * of a subprogram, so a caller reading R2 would see a
> + * stale value.
> + */
> + env->prog->jit_required = 1;
> + }
> + }
> }
>
> static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
...
Regarding the bot comment:
> Additionally, should jit_required be set for subprogs whose BTF prototypes
> are marked unreliable? btf_check_subprog_call() can flag a mismatch with
> prog->aux->func_info_aux[subprog].unreliable = true, and check_func_call()
> only aborts on -EFAULT, so a static callee with an aggregate return can
> load today without using the BTF return type. Subprogs subsequently deleted
> by bpf_opt_remove_dead_code() (which runs after this pass) also don't need
> jit_required set.
It seem to be correct. As prepare_func_exit() unconditionally copies
r0 and r2 to caller's frame, the verifier would conclude that r2 is
initialized; while interpreter won't copy it. One option is to force
nregs == 1 if there is no jit.
(Or derive this information not from BTF, I'll comment on that later).