LLVM 23 added support for returning a value in two registers for an __int128, or a struct/union whose size is greater than 8 but not more than 16 bytes. See LLVM patches [1] and [2]. Before LLVM 23 the BPF backend could not return these values at all. A by-value struct or union return (of any size) was rejected at compile time with: error: aggregate returns are not supported and an __int128 return failed later in the backend with: fatal error: error in backend: unable to allocate function return #1 Both are resolved in LLVM 23, which lowers such returns into the R0:R2 register pair. Model that pair at calls to global and static BPF subprograms and at kfunc calls: R2 is marked alongside R0 at the call, propagated out of a callee at its exit, and held to the same scalar-only and no-stack-pointer rules that already apply to R0. A struct returned by a kfunc must be composed of scalars, since its bytes reach the program as raw register contents and a pointer field would otherwise be laundered into a scalar. An extension program is the one caller of the convention that cannot take part in it: only R0 is checked at its exit, since that is the program exit code, so it has no way to hand back an upper half. Replacing a function whose return value is larger than 8 bytes is therefore rejected in btf_check_func_type_match(). That function compares btf_type->info, which carries no size for an int, so an 8 byte long and a 16 byte __int128 compared equal; sizes up to 8 bytes stay interchangeable, as they always have been, so this only rejects what the R0:R2 convention newly makes incompatible. [1] https://github.com/llvm/llvm-project/pull/190894 [2] https://github.com/llvm/llvm-project/pull/206876 Acked-by: Eduard Zingerman Signed-off-by: Yonghong Song --- kernel/bpf/btf.c | 6 ++++ kernel/bpf/verifier.c | 68 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 6967d48bba49..5e9f6a487524 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -7684,6 +7684,12 @@ static int btf_check_func_type_match(struct bpf_verifier_log *log, btf_type_str(t2), fn2); return -EINVAL; } + if (btf_type_has_size(t1) && (t1->size > 8 || t2->size > 8)) { + bpf_log(log, + "Return type of %s() has size %u and of %s() size %u, and a size above 8 bytes cannot be replaced\n", + fn1, t1->size, fn2, t2->size); + return -EINVAL; + } for (i = 0; i < nargs1; i++) { t1 = btf_type_skip_modifiers(btf1, args1[i].type, NULL); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 991782b402f2..d79f0c069795 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -415,6 +415,9 @@ static u32 ret_regs_cnt(u32 size) return size > 8 && size <= 16 ? 2 : 1; } +/* Registers holding a function return value, in order. See ret_regs_cnt(). */ +static const int ret_regs[] = { BPF_REG_0, BPF_REG_2 }; + static int bpf_compute_subprog_ret_regs(struct bpf_verifier_env *env) { const struct btf *btf = env->prog->aux->btf; @@ -9904,6 +9907,7 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, u16 callee_incoming, stack_arg_cnt; struct bpf_func_state *caller; int err, subprog, target_insn; + u32 i, nregs; target_insn = *insn_idx + insn->imm + 1; subprog = bpf_find_subprog(env, target_insn); @@ -9959,9 +9963,14 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, clear_caller_saved_regs(env, caller->regs); invalidate_outgoing_stack_args(env, cur_func(env)); - /* All non-void global functions return a 64-bit SCALAR_VALUE. */ + /* + * A non-void global function returns a 64-bit SCALAR_VALUE in + * R0, or a >8 byte SCALAR_VALUE in the R0:R2 register pair. + */ if (!returns_void) { - mark_reg_unknown(env, caller->regs, BPF_REG_0); + nregs = bpf_ret_reg_pair(env, subprog) ? 2 : 1; + for (i = 0; i < nregs; i++) + mark_reg_unknown(env, caller->regs, ret_regs[i]); bpf_diag_mod_end(env); } @@ -10321,11 +10330,15 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) struct bpf_func_state *caller, *callee; struct bpf_reg_state *r0; bool in_callback_fn; + u32 i, nregs; int err; callee = state->frame[state->curframe]; r0 = &callee->regs[BPF_REG_0]; - if (r0->type == PTR_TO_STACK) { + nregs = bpf_ret_reg_pair(env, callee->subprogno) ? 2 : 1; + for (i = 0; i < nregs; i++) { + if (callee->regs[ret_regs[i]].type != PTR_TO_STACK) + continue; /* technically it's ok to return caller's stack pointer * (or caller's caller's pointer) back to the caller, * since these pointers are valid. Only current stack @@ -10360,9 +10373,13 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) return -EFAULT; } } else { - /* return to the caller whatever r0 had in the callee */ + /* + * return to the caller whatever the callee had in the + * return register(s) + */ bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], r0, BPF_DIAG_MOD_WRITE); - caller->regs[BPF_REG_0] = *r0; + for (i = 0; i < nregs; i++) + caller->regs[ret_regs[i]] = callee->regs[ret_regs[i]]; bpf_diag_mod_end(env); } @@ -11297,6 +11314,19 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return 0; } +/* + * Mark the register(s) holding a @size byte kfunc return value as unknown + * scalars. Both halves of a register pair are treated the same way. + */ +static void mark_kfunc_ret_regs(struct bpf_verifier_env *env, + struct bpf_reg_state *regs, u32 size) +{ + u32 i, nregs = ret_regs_cnt(size); + + for (i = 0; i < nregs; i++) + mark_reg_unknown(env, regs, ret_regs[i]); +} + static bool is_kfunc_acquire(struct bpf_call_arg_meta *meta) { return meta->kfunc_flags & KF_ACQUIRE; @@ -13965,10 +13995,25 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, } if (btf_type_is_scalar(t)) { - mark_reg_unknown(env, regs, BPF_REG_0); + mark_kfunc_ret_regs(env, regs, t->size); if (meta.btf == btf_vmlinux && (meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock] || meta.func_id == special_kfunc_list[KF_bpf_res_spin_lock_irqsave])) __mark_reg_const_zero(env, ®s[BPF_REG_0]); + } else if (btf_type_is_struct(t)) { + /* + * The returned struct comes back as raw register bits modeled + * as an unknown scalar, so it must contain only scalars: + * otherwise a pointer field would be laundered into a scalar + * and escape provenance and reference tracking. + */ + if (!__btf_type_is_scalar_struct(env, desc_btf, t, 0)) { + verbose(env, + "kernel function %s returns %s %s that is not composed of scalars\n", + func_name, btf_type_str(t), + btf_name_by_offset(desc_btf, t->name_off)); + return -EINVAL; + } + mark_kfunc_ret_regs(env, regs, t->size); } else if (btf_type_is_ptr(t)) { ptr_type = btf_type_skip_modifiers(desc_btf, t->type, &ptr_type_id); err = check_special_kfunc(env, &meta, regs, insn_aux, ptr_type, desc_btf); @@ -17536,11 +17581,20 @@ static int check_global_subprog_return_code(struct bpf_verifier_env *env) { struct bpf_func_state *cur_frame = cur_func(env); u32 subprog = cur_frame->subprogno; + u32 i, nregs; + int err; if (subprog_returns_void(env, subprog)) return 0; - return check_global_ret_scalar_reg(env, BPF_REG_0); + nregs = bpf_ret_reg_pair(env, subprog) ? 2 : 1; + for (i = 0; i < nregs; i++) { + err = check_global_ret_scalar_reg(env, ret_regs[i]); + if (err) + return err; + } + + return 0; } /* Bitmask with 1s for all caller saved registers */ -- 2.53.0-Meta