From: Alexei Starovoitov Introduce BPF_JMP | BPF_CALL | BPF_X (opcode 0x8d) 'callx dst_reg' instruction: indirect call of bpf subprog with address in dst_reg. That's the encoding LLVM emits for calls via function pointer. src_reg, off, imm are reserved and must be zero. dst_reg must be PTR_TO_FUNC produced by ld_imm64 BPF_PSEUDO_FUNC. check_ld_imm() allows it for static subprogs only, so callx cannot call global subprogs or the main prog. Since every callee has its address taken by ld_imm64, add_subprogs() and check_cfg() see all of them before the main pass, and might_sleep, changes_pkt_data, might_throw of the callee are already merged into the subprog that takes the address. reg->subprogno is the callee. Verify callx as a direct call of that static subprog: split check_func_call() into check_static_func_call() that is shared with new check_func_callx(). Different paths through the same callx may call different subprogs. Arithmetic on PTR_TO_FUNC is allowed, so check that the pointer wasn't modified. Allow callx while holding a lock like direct calls of static subprogs. The interpreter doesn't support callx. Set jit_required and add bpf_jit_supports_callx() for JITs to opt in. No JIT does yet, so callx is still rejected. Print it as "callx rN" in the verifier log and xlated dump. Adjust "invalid call insn1" test_verifier test that used opcode 0x8d as unknown opcode. It fails with "R0 !read_ok" now. Signed-off-by: Alexei Starovoitov --- include/linux/filter.h | 1 + kernel/bpf/core.c | 7 + kernel/bpf/disasm.c | 5 +- kernel/bpf/verifier.c | 144 ++++++++++++++---- .../selftests/bpf/verifier/basic_call.c | 2 +- 5 files changed, 131 insertions(+), 28 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 422284b4fa96..4f0662e42897 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1237,6 +1237,7 @@ bool bpf_jit_inlines_helper_call(s32 imm); bool bpf_jit_supports_subprog_tailcalls(void); bool bpf_jit_supports_percpu_insn(void); bool bpf_jit_supports_kfunc_call(void); +bool bpf_jit_supports_callx(void); bool bpf_jit_supports_kfunc_ret_reg_pair(void); bool bpf_jit_supports_stack_args(void); bool bpf_jit_supports_arena_args(void); diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 227211166dcc..868350977f0c 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1831,6 +1831,7 @@ bool bpf_opcode_in_insntable(u8 code) [BPF_LD | BPF_IND | BPF_H] = true, [BPF_LD | BPF_IND | BPF_W] = true, [BPF_JMP | BPF_JA | BPF_X] = true, + [BPF_JMP | BPF_CALL | BPF_X] = true, [BPF_JMP | BPF_JCOND] = true, }; #undef BPF_INSN_3_TBL @@ -3287,6 +3288,12 @@ bool __weak bpf_jit_supports_kfunc_call(void) return false; } +/* Return TRUE if the JIT backend supports callx (indirect call) instruction. */ +bool __weak bpf_jit_supports_callx(void) +{ + return false; +} + bool __weak bpf_jit_supports_kfunc_ret_reg_pair(void) { return false; diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c index 3ce8d74b0e40..36d3228d7745 100644 --- a/kernel/bpf/disasm.c +++ b/kernel/bpf/disasm.c @@ -350,7 +350,10 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs, if (opcode == BPF_CALL) { char tmp[64]; - if (insn->src_reg == BPF_PSEUDO_CALL) { + if (BPF_SRC(insn->code) == BPF_X) { + verbose(cbs->private_data, "(%02x) callx r%d", + insn->code, insn->dst_reg); + } else if (insn->src_reg == BPF_PSEUDO_CALL) { verbose(cbs->private_data, "(%02x) call pc%s", insn->code, __func_get_name(cbs, insn, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 72ea662baea5..0d32d3921210 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10600,13 +10600,61 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins static int process_bpf_exit_full(struct bpf_verifier_env *env, bool *do_print_state, bool exception_exit); -static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, - int *insn_idx) +/* + * Call of a static subprog. The callee is verified in the context of + * the caller, hence set up a new frame and continue from the first + * instruction of the callee. + */ +static int check_static_func_call(struct bpf_verifier_env *env, int subprog, + int *insn_idx) { struct bpf_verifier_state *state = env->cur_state; struct bpf_subprog_info *caller_info; u16 callee_incoming, stack_arg_cnt; struct bpf_func_state *caller; + int err; + + caller = state->frame[state->curframe]; + + /* + * Track caller's total stack arg count (incoming + max outgoing). + * This is needed so the JIT knows how much stack arg space to allocate. + */ + caller_info = &env->subprog_info[caller->subprogno]; + callee_incoming = bpf_in_stack_arg_cnt(&env->subprog_info[subprog]); + stack_arg_cnt = bpf_in_stack_arg_cnt(caller_info) + callee_incoming; + if (stack_arg_cnt > caller_info->stack_arg_cnt) + caller_info->stack_arg_cnt = stack_arg_cnt; + + /* + * For regular function entry setup new frame and continue + * from that frame. + */ + err = setup_func_entry(env, subprog, *insn_idx, set_callee_state, state); + if (err) + return err; + + bpf_diag_record_scrub(env, &caller->regs[BPF_REG_0], BPF_DIAG_MOD_CALLER_SAVED); + clear_caller_saved_regs(env, caller->regs); + + /* and go analyze first insn of the callee */ + *insn_idx = env->subprog_info[subprog].start - 1; + + if (env->log.level & BPF_LOG_LEVEL) { + verbose(env, "caller:\n"); + print_verifier_state(env, state, caller->frameno, true); + verbose(env, "callee:\n"); + print_verifier_state(env, state, state->curframe, true); + } + + return 0; +} + +static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, + int *insn_idx) +{ + struct bpf_verifier_state *state = env->cur_state; + struct bpf_func_state *caller; int err, subprog, target_insn; u32 i, nregs; @@ -10695,37 +10743,70 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn, return 0; } - /* - * Track caller's total stack arg count (incoming + max outgoing). - * This is needed so the JIT knows how much stack arg space to allocate. - */ - caller_info = &env->subprog_info[caller->subprogno]; - callee_incoming = bpf_in_stack_arg_cnt(&env->subprog_info[subprog]); - stack_arg_cnt = bpf_in_stack_arg_cnt(caller_info) + callee_incoming; - if (stack_arg_cnt > caller_info->stack_arg_cnt) - caller_info->stack_arg_cnt = stack_arg_cnt; + return check_static_func_call(env, subprog, insn_idx); +} - /* for regular function entry setup new frame and continue - * from that frame. - */ - err = setup_func_entry(env, subprog, *insn_idx, set_callee_state, state); +/* + * callx dst_reg: call a bpf subprog whose address is in dst_reg. + * + * The address of a subprog is loaded into a register by ld_imm64 with + * src_reg == BPF_PSEUDO_FUNC, which is allowed for static subprogs only. + * Hence all possible callees of callx are discovered by add_subprogs() and + * are reachable in the control flow graph before the main verification pass + * begins. PTR_TO_FUNC register identifies the callee, so from here on callx + * is verified as a direct call of that static subprog. + */ +static int check_func_callx(struct bpf_verifier_env *env, struct bpf_insn *insn, + int *insn_idx) +{ + struct bpf_func_state *caller = cur_func(env); + struct bpf_reg_state *reg; + const char *reason; + int err, subprog; + + err = check_reg_arg(env, insn->dst_reg, SRC_OP); if (err) return err; - bpf_diag_record_scrub(env, &caller->regs[BPF_REG_0], BPF_DIAG_MOD_CALLER_SAVED); - clear_caller_saved_regs(env, caller->regs); + reg = reg_state(env, insn->dst_reg); + if (reg->type != PTR_TO_FUNC) { + verbose(env, "R%d has type %s, expected func\n", insn->dst_reg, + reg_type_str(env, reg->type)); + reason = bpf_diag_fmt( + env, "R%d holds %s, but callx can only call through the address of a static BPF function.", + insn->dst_reg, bpf_diag_reg_type_plain(env, reg->type)); + bpf_diag_register_type( + env, *insn_idx, insn->dst_reg, "indirect call through a non-function pointer", reason, + "Load the address of a static BPF function into the register before callx."); + return -EACCES; + } - /* and go analyze first insn of the callee */ - *insn_idx = env->subprog_info[subprog].start - 1; + /* + * Arithmetic on PTR_TO_FUNC is allowed, but only unmodified address + * of a subprog can be called. + */ + err = check_ptr_off_reg(env, reg, insn->dst_reg); + if (err) + return err; - if (env->log.level & BPF_LOG_LEVEL) { - verbose(env, "caller:\n"); - print_verifier_state(env, state, caller->frameno, true); - verbose(env, "callee:\n"); - print_verifier_state(env, state, state->curframe, true); + /* there is no support for callx in the interpreter */ + if (!env->prog->jit_requested) { + verbose(env, "JIT is required to use callx\n"); + return -EOPNOTSUPP; } + if (!bpf_jit_supports_callx()) { + verbose(env, "JIT doesn't support callx\n"); + return -EOPNOTSUPP; + } + env->prog->jit_required = true; - return 0; + /* check_ld_imm() allows to take the address of static subprogs only */ + subprog = reg->subprogno; + err = btf_check_subprog_call(env, subprog, caller->regs); + if (err == -EFAULT) + return err; + + return check_static_func_call(env, subprog, insn_idx); } int map_set_for_each_callback_args(struct bpf_verifier_env *env, @@ -18725,7 +18806,8 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state) env->jmps_processed++; if (opcode == BPF_CALL) { - if (env->cur_state->active_locks) { + /* similar to static subprog calls callx is allowed under a lock */ + if (env->cur_state->active_locks && !bpf_is_callx(insn)) { if ((insn->src_reg == BPF_REG_0 && insn->imm != BPF_FUNC_spin_unlock && insn->imm != BPF_FUNC_kptr_xchg) || @@ -18743,6 +18825,8 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state) mark_reg_scratched(env, BPF_REG_0); if (bpf_in_stack_arg_cnt(&env->subprog_info[cur_func(env)->subprogno])) cur_func(env)->no_stack_arg_load = true; + if (bpf_is_callx(insn)) + return check_func_callx(env, insn, &env->insn_idx); if (insn->src_reg == BPF_PSEUDO_CALL) return check_func_call(env, insn, &env->insn_idx); if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) @@ -19509,6 +19593,14 @@ static int check_jmp_fields(struct bpf_verifier_env *env, struct bpf_insn *insn) switch (opcode) { case BPF_CALL: + if (bpf_is_callx(insn)) { + /* callx dst_reg */ + if (insn->src_reg != BPF_REG_0 || insn->imm != 0 || insn->off != 0) { + verbose(env, "BPF_CALL|BPF_X uses reserved fields\n"); + return -EINVAL; + } + return 0; + } if (BPF_SRC(insn->code) != BPF_K || (insn->src_reg != BPF_PSEUDO_KFUNC_CALL && insn->off != 0) || (insn->src_reg != BPF_REG_0 && insn->src_reg != BPF_PSEUDO_CALL && diff --git a/tools/testing/selftests/bpf/verifier/basic_call.c b/tools/testing/selftests/bpf/verifier/basic_call.c index a8c6ab4c1622..0f93c4551f23 100644 --- a/tools/testing/selftests/bpf/verifier/basic_call.c +++ b/tools/testing/selftests/bpf/verifier/basic_call.c @@ -4,7 +4,7 @@ BPF_RAW_INSN(BPF_JMP | BPF_CALL | BPF_X, 0, 0, 0, 0), BPF_EXIT_INSN(), }, - .errstr = "unknown opcode 8d", + .errstr = "R0 !read_ok", .result = REJECT, }, { -- 2.55.0