Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bpf: abstract away global subprog arg preparation logic from reg state setup

btf_prepare_func_args() is used to understand expectations and
restrictions on global subprog arguments. But current implementation is
hard to extend, as it intermixes BTF-based func prototype parsing and
interpretation logic with setting up register state at subprog entry.

Worse still, those registers are not completely set up inside
btf_prepare_func_args(), requiring some more logic later in
do_check_common(). Like calling mark_reg_unknown() and similar
initialization operations.

This intermixing of BTF interpretation and register state setup is
problematic. First, it causes duplication of BTF parsing logic for global
subprog verification (to set up initial state of global subprog) and
global subprog call sites analysis (when we need to check that whatever
is being passed into global subprog matches expectations), performed in
btf_check_subprog_call().

Given we want to extend global func argument with tags later, this
duplication is problematic. So refactor btf_prepare_func_args() to do
only BTF-based func proto and args parsing, returning high-level
argument "expectations" only, with no regard to specifics of register
state. I.e., if it's a context argument, instead of setting register
state to PTR_TO_CTX, we return ARG_PTR_TO_CTX enum for that argument as
"an argument specification" for further processing inside
do_check_common(). Similarly for SCALAR arguments, PTR_TO_MEM, etc.

This allows to reuse btf_prepare_func_args() in following patches at
global subprog call site analysis time. It also keeps register setup
code consistently in one place, do_check_common().

Besides all this, we cache this argument specs information inside
env->subprog_info, eliminating the need to redo these potentially
expensive BTF traversals, especially if BPF program's BTF is big and/or
there are lots of global subprog calls.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231215011334.2307144-2-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

authored by

Andrii Nakryiko and committed by
Alexei Starovoitov
4ba1d0f2 c337f237

+65 -37
+1 -2
include/linux/bpf.h
··· 2470 2470 struct bpf_reg_state *regs); 2471 2471 int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog, 2472 2472 struct bpf_reg_state *regs); 2473 - int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, 2474 - struct bpf_reg_state *reg, u32 *nargs); 2473 + int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog); 2475 2474 int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog, 2476 2475 struct btf *btf, const struct btf_type *t); 2477 2476 const char *btf_find_decl_tag_value(const struct btf *btf, const struct btf_type *pt,
+16
include/linux/bpf_verifier.h
··· 606 606 607 607 #define BPF_MAX_SUBPROGS 256 608 608 609 + struct bpf_subprog_arg_info { 610 + enum bpf_arg_type arg_type; 611 + union { 612 + u32 mem_size; 613 + }; 614 + }; 615 + 609 616 struct bpf_subprog_info { 610 617 /* 'start' has to be the first field otherwise find_subprog() won't work */ 611 618 u32 start; /* insn idx of function entry point */ ··· 624 617 bool is_cb: 1; 625 618 bool is_async_cb: 1; 626 619 bool is_exception_cb: 1; 620 + bool args_cached: 1; 621 + 622 + u8 arg_cnt; 623 + struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS]; 627 624 }; 628 625 629 626 struct bpf_verifier_env; ··· 737 726 */ 738 727 char tmp_str_buf[TMP_STR_BUF_LEN]; 739 728 }; 729 + 730 + static inline struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog) 731 + { 732 + return &env->subprog_info[subprog]; 733 + } 740 734 741 735 __printf(2, 0) void bpf_verifier_vlog(struct bpf_verifier_log *log, 742 736 const char *fmt, va_list args);
+20 -18
kernel/bpf/btf.c
··· 6948 6948 return err; 6949 6949 } 6950 6950 6951 - /* Convert BTF of a function into bpf_reg_state if possible 6951 + /* Process BTF of a function to produce high-level expectation of function 6952 + * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information 6953 + * is cached in subprog info for reuse. 6952 6954 * Returns: 6953 6955 * EFAULT - there is a verifier bug. Abort verification. 6954 6956 * EINVAL - cannot convert BTF. 6955 - * 0 - Successfully converted BTF into bpf_reg_state 6956 - * (either PTR_TO_CTX or SCALAR_VALUE). 6957 + * 0 - Successfully processed BTF and constructed argument expectations. 6957 6958 */ 6958 - int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog, 6959 - struct bpf_reg_state *regs, u32 *arg_cnt) 6959 + int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) 6960 6960 { 6961 + struct bpf_subprog_info *sub = subprog_info(env, subprog); 6961 6962 struct bpf_verifier_log *log = &env->log; 6962 6963 struct bpf_prog *prog = env->prog; 6963 6964 enum bpf_prog_type prog_type = prog->type; ··· 6967 6966 const struct btf_type *t, *ref_t; 6968 6967 u32 i, nargs, btf_id; 6969 6968 const char *tname; 6969 + 6970 + if (sub->args_cached) 6971 + return 0; 6970 6972 6971 6973 if (!prog->aux->func_info || 6972 6974 prog->aux->func_info_aux[subprog].linkage != BTF_FUNC_GLOBAL) { ··· 6994 6990 } 6995 6991 tname = btf_name_by_offset(btf, t->name_off); 6996 6992 6997 - if (log->level & BPF_LOG_LEVEL) 6998 - bpf_log(log, "Validating %s() func#%d...\n", 6999 - tname, subprog); 7000 - 7001 6993 if (prog->aux->func_info_aux[subprog].unreliable) { 7002 6994 bpf_log(log, "Verifier bug in function %s()\n", tname); 7003 6995 return -EFAULT; ··· 7013 7013 tname, nargs, MAX_BPF_FUNC_REG_ARGS); 7014 7014 return -EINVAL; 7015 7015 } 7016 - *arg_cnt = nargs; 7017 7016 /* check that function returns int, exception cb also requires this */ 7018 7017 t = btf_type_by_id(btf, t->type); 7019 7018 while (btf_type_is_modifier(t)) ··· 7027 7028 * Only PTR_TO_CTX and SCALAR are supported atm. 7028 7029 */ 7029 7030 for (i = 0; i < nargs; i++) { 7030 - struct bpf_reg_state *reg = &regs[i + 1]; 7031 - 7032 7031 t = btf_type_by_id(btf, args[i].type); 7033 7032 while (btf_type_is_modifier(t)) 7034 7033 t = btf_type_by_id(btf, t->type); 7035 7034 if (btf_type_is_int(t) || btf_is_any_enum(t)) { 7036 - reg->type = SCALAR_VALUE; 7035 + sub->args[i].arg_type = ARG_ANYTHING; 7037 7036 continue; 7038 7037 } 7039 7038 if (btf_type_is_ptr(t)) { 7039 + u32 mem_size; 7040 + 7040 7041 if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) { 7041 - reg->type = PTR_TO_CTX; 7042 + sub->args[i].arg_type = ARG_PTR_TO_CTX; 7042 7043 continue; 7043 7044 } 7044 7045 7045 7046 t = btf_type_skip_modifiers(btf, t->type, NULL); 7046 7047 7047 - ref_t = btf_resolve_size(btf, t, &reg->mem_size); 7048 + ref_t = btf_resolve_size(btf, t, &mem_size); 7048 7049 if (IS_ERR(ref_t)) { 7049 7050 bpf_log(log, 7050 7051 "arg#%d reference type('%s %s') size cannot be determined: %ld\n", ··· 7053 7054 return -EINVAL; 7054 7055 } 7055 7056 7056 - reg->type = PTR_TO_MEM | PTR_MAYBE_NULL; 7057 - reg->id = ++env->id_gen; 7058 - 7057 + sub->args[i].arg_type = ARG_PTR_TO_MEM_OR_NULL; 7058 + sub->args[i].mem_size = mem_size; 7059 7059 continue; 7060 7060 } 7061 7061 bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n", 7062 7062 i, btf_type_str(t), tname); 7063 7063 return -EINVAL; 7064 7064 } 7065 + 7066 + sub->arg_cnt = nargs; 7067 + sub->args_cached = true; 7068 + 7065 7069 return 0; 7066 7070 } 7067 7071
+28 -17
kernel/bpf/verifier.c
··· 442 442 return &env->prog->aux->func_info_aux[subprog]; 443 443 } 444 444 445 - static struct bpf_subprog_info *subprog_info(struct bpf_verifier_env *env, int subprog) 446 - { 447 - return &env->subprog_info[subprog]; 448 - } 449 - 450 445 static void mark_subprog_exc_cb(struct bpf_verifier_env *env, int subprog) 451 446 { 452 447 struct bpf_subprog_info *info = subprog_info(env, subprog); ··· 19932 19937 19933 19938 regs = state->frame[state->curframe]->regs; 19934 19939 if (subprog || env->prog->type == BPF_PROG_TYPE_EXT) { 19935 - u32 nargs; 19940 + struct bpf_subprog_info *sub = subprog_info(env, subprog); 19941 + const char *sub_name = subprog_name(env, subprog); 19942 + struct bpf_subprog_arg_info *arg; 19943 + struct bpf_reg_state *reg; 19936 19944 19937 - ret = btf_prepare_func_args(env, subprog, regs, &nargs); 19945 + verbose(env, "Validating %s() func#%d...\n", sub_name, subprog); 19946 + ret = btf_prepare_func_args(env, subprog); 19938 19947 if (ret) 19939 19948 goto out; 19949 + 19940 19950 if (subprog_is_exc_cb(env, subprog)) { 19941 19951 state->frame[0]->in_exception_callback_fn = true; 19942 19952 /* We have already ensured that the callback returns an integer, just 19943 19953 * like all global subprogs. We need to determine it only has a single 19944 19954 * scalar argument. 19945 19955 */ 19946 - if (nargs != 1 || regs[BPF_REG_1].type != SCALAR_VALUE) { 19956 + if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_ANYTHING) { 19947 19957 verbose(env, "exception cb only supports single integer argument\n"); 19948 19958 ret = -EINVAL; 19949 19959 goto out; 19950 19960 } 19951 19961 } 19952 - for (i = BPF_REG_1; i <= BPF_REG_5; i++) { 19953 - if (regs[i].type == PTR_TO_CTX) 19954 - mark_reg_known_zero(env, regs, i); 19955 - else if (regs[i].type == SCALAR_VALUE) 19956 - mark_reg_unknown(env, regs, i); 19957 - else if (base_type(regs[i].type) == PTR_TO_MEM) { 19958 - const u32 mem_size = regs[i].mem_size; 19962 + for (i = BPF_REG_1; i <= sub->arg_cnt; i++) { 19963 + arg = &sub->args[i - BPF_REG_1]; 19964 + reg = &regs[i]; 19959 19965 19966 + if (arg->arg_type == ARG_PTR_TO_CTX) { 19967 + reg->type = PTR_TO_CTX; 19960 19968 mark_reg_known_zero(env, regs, i); 19961 - regs[i].mem_size = mem_size; 19962 - regs[i].id = ++env->id_gen; 19969 + } else if (arg->arg_type == ARG_ANYTHING) { 19970 + reg->type = SCALAR_VALUE; 19971 + mark_reg_unknown(env, regs, i); 19972 + } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) { 19973 + reg->type = PTR_TO_MEM; 19974 + if (arg->arg_type & PTR_MAYBE_NULL) 19975 + reg->type |= PTR_MAYBE_NULL; 19976 + mark_reg_known_zero(env, regs, i); 19977 + reg->mem_size = arg->mem_size; 19978 + reg->id = ++env->id_gen; 19979 + } else { 19980 + WARN_ONCE(1, "BUG: unhandled arg#%d type %d\n", 19981 + i - BPF_REG_1, arg->arg_type); 19982 + ret = -EFAULT; 19983 + goto out; 19963 19984 } 19964 19985 } 19965 19986 } else {