| Message ID | 20230214190551.2264057-1-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [v3,bpf-next] bpf: Refactor release_regno searching logic | expand |
On 2/14/23 2:05 PM, Dave Marchevsky wrote: > Currently the ref_obj_id and OBJ_RELEASE searching is done in the code > that examines each individual arg (check_func_arg for helpers and > check_kfunc_args inner loop for kfuncs). This patch pulls out this > searching to occur before individual arg type handling, resulting in a > cleaner separation of logic and shared logic between kfuncs and helpers. > > The logic for this searching is already very similar between kfuncs and > helpers: > > Kfuncs: > * Function-level KF_RELEASE flag indicates that the kfunc releases > some previously-acquired arg > * Verifier searches through arg regs to find those with ref_obj_id set > * One such arg reg is selected. If multiple arg regs have ref_obj_id > set, the last one (by regno) is chosen to be released > > Helpers: > * OBJ_RELEASE is used in function proto to tag a particular arg as the > one that should be released > * There can only be one such tagged arg > * Verifier confirms that only one arg reg has ref_obj_id set and that > that reg matches expected OBJ_RELEASE arg > * If OBJ_RELEASE arg doesn't have a matching ref_obj_id arg reg, or > some other arg reg has ref_obj_id, it's an invalid state > > It's a long-term goal to merge as much kfunc and helper logic as > possible. Merging the similar functionality here is a small step in that > direction. > > Two new helper functions are added: > * args_find_ref_obj_id_regno > * For helpers and kfuncs. Searches through arg regs to find > ref_obj_id reg and returns its regno. > > * helper_proto_find_release_arg_regno > * For helpers only. Searches through fn proto args to find the > OBJ_RELEASE arg and returns the corresponding regno. > > The refactoring strives to keep failure logic and error messages > unchanged. However, because the release arg searching is now done before > any arg-specific type checking, verifier states that are invalid due to > both invalid release arg state _and_ some type- or helper-specific > checking logic might see the release arg-related error message first, > when previously verification would fail for the other reason. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > v2 -> v3: Whoops, didn't include v2 link. Here it is: https://lore.kernel.org/bpf/20230131171038.2648165-1-davemarchevsky@fb.com/ > * Edit patch summary for clarity > * Correct err_multi comment in args_find_ref_obj_id_regno doc string > * Rebase onto latest bpf-next: 'Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"' > > v1 -> v2: https://lore.kernel.org/bpf/20230121002417.1684602-1-davemarchevsky@fb.com/ > * Fix uninitialized variable complaint (kernel test bot) > * Add err_multi param to args_find_ref_obj_id_regno - kfunc arg reg > checking wasn't erroring if multiple ref_obj_id arg regs were found, > retain this behavior > > v0 -> v1: https://lore.kernel.org/bpf/20221217082506.1570898-2-davemarchevsky@fb.com/ > * Remove allow_multi from args_find_ref_obj_id_regno, no need to > support multiple ref_obj_id arg regs > * No need to use temp variable 'i' to count nargs (David) > * Proper formatting of function-level comments on newly-added helpers (David) > > kernel/bpf/verifier.c | 220 +++++++++++++++++++++++++++++------------- > 1 file changed, 153 insertions(+), 67 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 21e08c111702..c0d01085f44f 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -6735,48 +6735,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > return err; > > skip_type_check: > - if (arg_type_is_release(arg_type)) { > - if (arg_type_is_dynptr(arg_type)) { > - struct bpf_func_state *state = func(env, reg); > - int spi; > - > - /* Only dynptr created on stack can be released, thus > - * the get_spi and stack state checks for spilled_ptr > - * should only be done before process_dynptr_func for > - * PTR_TO_STACK. > - */ > - if (reg->type == PTR_TO_STACK) { > - spi = dynptr_get_spi(env, reg); > - if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) { > - verbose(env, "arg %d is an unacquired reference\n", regno); > - return -EINVAL; > - } > - } else { > - verbose(env, "cannot release unowned const bpf_dynptr\n"); > - return -EINVAL; > - } > - } else if (!reg->ref_obj_id && !register_is_null(reg)) { > - verbose(env, "R%d must be referenced when passed to release function\n", > - regno); > - return -EINVAL; > - } > - if (meta->release_regno) { > - verbose(env, "verifier internal error: more than one release argument\n"); > - return -EFAULT; > - } > - meta->release_regno = regno; > - } > - > - if (reg->ref_obj_id) { > - if (meta->ref_obj_id) { > - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", > - regno, reg->ref_obj_id, > - meta->ref_obj_id); > - return -EFAULT; > - } > - meta->ref_obj_id = reg->ref_obj_id; > - } > - > switch (base_type(arg_type)) { > case ARG_CONST_MAP_PTR: > /* bpf_map_xxx(map_ptr) call: remember that map_ptr */ > @@ -6891,6 +6849,26 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > err = check_mem_size_reg(env, reg, regno, true, meta); > break; > case ARG_PTR_TO_DYNPTR: > + if (meta->release_regno == regno) { > + struct bpf_func_state *state = func(env, reg); > + int spi; > + > + /* Only dynptr created on stack can be released, thus > + * the get_spi and stack state checks for spilled_ptr > + * should only be done before process_dynptr_func for > + * PTR_TO_STACK. > + */ > + if (reg->type == PTR_TO_STACK) { > + spi = dynptr_get_spi(env, reg); > + if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) { > + verbose(env, "arg %d is an unacquired reference\n", regno); > + return -EINVAL; > + } > + } else { > + verbose(env, "cannot release unowned const bpf_dynptr\n"); > + return -EINVAL; > + } > + } > err = process_dynptr_func(env, regno, arg_type, meta); > if (err) > return err; > @@ -8104,10 +8082,95 @@ static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno > state->callback_subprogno == subprogno); > } > > +/** > + * args_find_ref_obj_id_regno() - Find regno that should become meta->ref_obj_id > + * @env: Verifier env > + * @regs: Regs to search for ref_obj_id > + * @nargs: Number of arg regs to search > + * @err_multi: Should this function error if multiple ref_obj_id args found > + * > + * Call arg meta's ref_obj_id is used to either: > + * * For release funcs, keep track of ref that needs to be released > + * * For other funcs, keep track of ref that needs to be propagated to retval > + * > + * Find the arg regno with nonzero ref_obj_id > + * > + * If err_multi is false and multiple ref_obj_id arg regs are seen, regno of the > + * last one is returned > + * > + * Return: > + * * On success, regno that should become meta->ref_obj_id (regno > 0 since > + * BPF_REG_1 is first arg > + * * 0 if no arg had ref_obj_id set > + * * -err if some invalid arg reg state > + */ > +static int args_find_ref_obj_id_regno(struct bpf_verifier_env *env, struct bpf_reg_state *regs, > + u32 nargs, bool err_multi) > +{ > + struct bpf_reg_state *reg; > + u32 i, regno, found_regno = 0; > + > + for (i = 0; i < nargs; i++) { > + regno = i + BPF_REG_1; > + reg = ®s[regno]; > + > + if (!reg->ref_obj_id) > + continue; > + > + if (found_regno && err_multi) { > + verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", > + regno, reg->ref_obj_id, regs[found_regno].ref_obj_id); > + return -EFAULT; > + } > + > + found_regno = regno; > + } > + > + return found_regno; > +} > + > +/** > + * helper_proto_find_release_arg_regno() - Find OBJ_RELEASE arg in func proto > + * @env: Verifier env > + * @fn: Func proto to search for OBJ_RELEASE > + * @nargs: Number of arg specs to search > + * > + * For helpers, to determine which arg reg should be released, loop through > + * func proto arg specification to find arg with OBJ_RELEASE > + * > + * Return: > + * * On success, regno of single OBJ_RELEASE arg > + * * 0 if no arg in the proto was OBJ_RELEASE > + * * -err if some invalid func proto state > + */ > +static int helper_proto_find_release_arg_regno(struct bpf_verifier_env *env, > + const struct bpf_func_proto *fn, u32 nargs) > +{ > + enum bpf_arg_type arg_type; > + int i, release_regno = 0; > + > + for (i = 0; i < nargs; i++) { > + arg_type = fn->arg_type[i]; > + > + if (!arg_type_is_release(arg_type)) > + continue; > + > + if (release_regno) { > + verbose(env, "verifier internal error: more than one release argument\n"); > + return -EFAULT; > + } > + > + release_regno = i + BPF_REG_1; > + } > + > + return release_regno; > +} > + > static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > int *insn_idx_p) > { > enum bpf_prog_type prog_type = resolve_prog_type(env->prog); > + int i, err, func_id, nargs, release_regno, ref_regno; > const struct bpf_func_proto *fn = NULL; > enum bpf_return_type ret_type; > enum bpf_type_flag ret_flag; > @@ -8115,7 +8178,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > struct bpf_call_arg_meta meta; > int insn_idx = *insn_idx_p; > bool changes_data; > - int i, err, func_id; > > /* find function prototype */ > func_id = insn->imm; > @@ -8179,8 +8241,37 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > } > > meta.func_id = func_id; > + regs = cur_regs(env); > + > + /* find actual arg count */ > + for (nargs = 0; nargs < MAX_BPF_FUNC_REG_ARGS; nargs++) > + if (fn->arg_type[nargs] == ARG_DONTCARE) > + break; > + > + release_regno = helper_proto_find_release_arg_regno(env, fn, nargs); > + if (release_regno < 0) > + return release_regno; > + > + ref_regno = args_find_ref_obj_id_regno(env, regs, nargs, true); > + if (ref_regno < 0) > + return ref_regno; > + else if (ref_regno > 0) > + meta.ref_obj_id = regs[ref_regno].ref_obj_id; > + > + if (release_regno > 0) { > + if (!regs[release_regno].ref_obj_id && > + !register_is_null(®s[release_regno]) && > + !arg_type_is_dynptr(fn->arg_type[release_regno - BPF_REG_1])) { > + verbose(env, "R%d must be referenced when passed to release function\n", > + release_regno); > + return -EINVAL; > + } > + > + meta.release_regno = release_regno; > + } > + > /* check args */ > - for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { > + for (i = 0; i < nargs; i++) { > err = check_func_arg(env, i, &meta, fn); > if (err) > return err; > @@ -8204,8 +8295,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > return err; > } > > - regs = cur_regs(env); > - > /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot > * be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr > * is safe to do directly. > @@ -9442,10 +9531,11 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env, > static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta) > { > const char *func_name = meta->func_name, *ref_tname; > + struct bpf_reg_state *regs = cur_regs(env); > const struct btf *btf = meta->btf; > const struct btf_param *args; > + int ret, ref_regno; > u32 i, nargs; > - int ret; > > args = (const struct btf_param *)(meta->func_proto + 1); > nargs = btf_type_vlen(meta->func_proto); > @@ -9455,17 +9545,31 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EINVAL; > } > > + ref_regno = args_find_ref_obj_id_regno(env, cur_regs(env), nargs, false); > + if (ref_regno < 0) { > + return ref_regno; > + } else if (!ref_regno && is_kfunc_release(meta)) { > + verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", > + func_name); > + return -EINVAL; > + } > + > + meta->ref_obj_id = regs[ref_regno].ref_obj_id; > + if (is_kfunc_release(meta)) > + meta->release_regno = ref_regno; > + > /* Check that BTF function arguments match actual types that the > * verifier sees. > */ > for (i = 0; i < nargs; i++) { > - struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[i + 1]; > const struct btf_type *t, *ref_t, *resolve_ret; > enum bpf_arg_type arg_type = ARG_DONTCARE; > u32 regno = i + 1, ref_id, type_size; > bool is_ret_buf_sz = false; > + struct bpf_reg_state *reg; > int kf_arg_type; > > + reg = ®s[regno]; > t = btf_type_skip_modifiers(btf, args[i].type, NULL); > > if (is_kfunc_arg_ignore(btf, &args[i])) > @@ -9528,18 +9632,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EACCES; > } > > - if (reg->ref_obj_id) { > - if (is_kfunc_release(meta) && meta->ref_obj_id) { > - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", > - regno, reg->ref_obj_id, > - meta->ref_obj_id); > - return -EFAULT; > - } > - meta->ref_obj_id = reg->ref_obj_id; > - if (is_kfunc_release(meta)) > - meta->release_regno = regno; > - } > - > ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); > ref_tname = btf_name_by_offset(btf, ref_t->name_off); > > @@ -9585,7 +9677,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EFAULT; > } > > - if (is_kfunc_release(meta) && reg->ref_obj_id) > + if (is_kfunc_release(meta) && regno == meta->release_regno) > arg_type |= OBJ_RELEASE; > ret = check_func_arg_reg_off(env, reg, regno, arg_type); > if (ret < 0) > @@ -9747,12 +9839,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > } > } > > - if (is_kfunc_release(meta) && !meta->release_regno) { > - verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", > - func_name); > - return -EINVAL; > - } > - > return 0; > } >
On Tue, Feb 14, 2023 at 11:17 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > Currently the ref_obj_id and OBJ_RELEASE searching is done in the code > that examines each individual arg (check_func_arg for helpers and > check_kfunc_args inner loop for kfuncs). This patch pulls out this > searching to occur before individual arg type handling, resulting in a > cleaner separation of logic and shared logic between kfuncs and helpers. > > The logic for this searching is already very similar between kfuncs and > helpers: > > Kfuncs: > * Function-level KF_RELEASE flag indicates that the kfunc releases > some previously-acquired arg > * Verifier searches through arg regs to find those with ref_obj_id set > * One such arg reg is selected. If multiple arg regs have ref_obj_id > set, the last one (by regno) is chosen to be released > > Helpers: > * OBJ_RELEASE is used in function proto to tag a particular arg as the > one that should be released > * There can only be one such tagged arg > * Verifier confirms that only one arg reg has ref_obj_id set and that > that reg matches expected OBJ_RELEASE arg > * If OBJ_RELEASE arg doesn't have a matching ref_obj_id arg reg, or > some other arg reg has ref_obj_id, it's an invalid state > > It's a long-term goal to merge as much kfunc and helper logic as > possible. Merging the similar functionality here is a small step in that > direction. > > Two new helper functions are added: > * args_find_ref_obj_id_regno > * For helpers and kfuncs. Searches through arg regs to find > ref_obj_id reg and returns its regno. > > * helper_proto_find_release_arg_regno > * For helpers only. Searches through fn proto args to find the > OBJ_RELEASE arg and returns the corresponding regno. > > The refactoring strives to keep failure logic and error messages > unchanged. However, because the release arg searching is now done before > any arg-specific type checking, verifier states that are invalid due to > both invalid release arg state _and_ some type- or helper-specific > checking logic might see the release arg-related error message first, > when previously verification would fail for the other reason. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > v2 -> v3: > * Edit patch summary for clarity > * Correct err_multi comment in args_find_ref_obj_id_regno doc string > * Rebase onto latest bpf-next: 'Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"' > > v1 -> v2: https://lore.kernel.org/bpf/20230121002417.1684602-1-davemarchevsky@fb.com/ > * Fix uninitialized variable complaint (kernel test bot) > * Add err_multi param to args_find_ref_obj_id_regno - kfunc arg reg > checking wasn't erroring if multiple ref_obj_id arg regs were found, > retain this behavior > > v0 -> v1: https://lore.kernel.org/bpf/20221217082506.1570898-2-davemarchevsky@fb.com/ > * Remove allow_multi from args_find_ref_obj_id_regno, no need to > support multiple ref_obj_id arg regs > * No need to use temp variable 'i' to count nargs (David) > * Proper formatting of function-level comments on newly-added helpers (David) > > kernel/bpf/verifier.c | 220 +++++++++++++++++++++++++++++------------- > 1 file changed, 153 insertions(+), 67 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 21e08c111702..c0d01085f44f 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -6735,48 +6735,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, > return err; > [...] > static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > int *insn_idx_p) > { > enum bpf_prog_type prog_type = resolve_prog_type(env->prog); > + int i, err, func_id, nargs, release_regno, ref_regno; > const struct bpf_func_proto *fn = NULL; > enum bpf_return_type ret_type; > enum bpf_type_flag ret_flag; > @@ -8115,7 +8178,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > struct bpf_call_arg_meta meta; > int insn_idx = *insn_idx_p; > bool changes_data; > - int i, err, func_id; > > /* find function prototype */ > func_id = insn->imm; > @@ -8179,8 +8241,37 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > } > > meta.func_id = func_id; > + regs = cur_regs(env); > + > + /* find actual arg count */ > + for (nargs = 0; nargs < MAX_BPF_FUNC_REG_ARGS; nargs++) > + if (fn->arg_type[nargs] == ARG_DONTCARE) > + break; > + > + release_regno = helper_proto_find_release_arg_regno(env, fn, nargs); > + if (release_regno < 0) > + return release_regno; > + > + ref_regno = args_find_ref_obj_id_regno(env, regs, nargs, true); > + if (ref_regno < 0) > + return ref_regno; > + else if (ref_regno > 0) > + meta.ref_obj_id = regs[ref_regno].ref_obj_id; nit: I think it's easier to read if this ref_regno logic gets moved below the release_regno logic, so that all the release_regno logic is together > + > + if (release_regno > 0) { > + if (!regs[release_regno].ref_obj_id && > + !register_is_null(®s[release_regno]) && > + !arg_type_is_dynptr(fn->arg_type[release_regno - BPF_REG_1])) { > + verbose(env, "R%d must be referenced when passed to release function\n", > + release_regno); > + return -EINVAL; > + } > + > + meta.release_regno = release_regno; > + } > + > /* check args */ > - for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { > + for (i = 0; i < nargs; i++) { > err = check_func_arg(env, i, &meta, fn); > if (err) > return err; > @@ -8204,8 +8295,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn > return err; > } > > - regs = cur_regs(env); > - > /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot > * be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr > * is safe to do directly. > @@ -9442,10 +9531,11 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env, > static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta) > { > const char *func_name = meta->func_name, *ref_tname; > + struct bpf_reg_state *regs = cur_regs(env); > const struct btf *btf = meta->btf; > const struct btf_param *args; > + int ret, ref_regno; > u32 i, nargs; > - int ret; > > args = (const struct btf_param *)(meta->func_proto + 1); > nargs = btf_type_vlen(meta->func_proto); > @@ -9455,17 +9545,31 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EINVAL; > } > > + ref_regno = args_find_ref_obj_id_regno(env, cur_regs(env), nargs, false); nit: I think we can just pass in "regs" as the 2nd arg > + if (ref_regno < 0) { > + return ref_regno; > + } else if (!ref_regno && is_kfunc_release(meta)) { > + verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", > + func_name); > + return -EINVAL; > + } > + > + meta->ref_obj_id = regs[ref_regno].ref_obj_id; > + if (is_kfunc_release(meta)) > + meta->release_regno = ref_regno; > + I think we also need to check that if the kfunc is a release func then there can't be more than one arg reg with a set ref_obj_id (the earlier call to args_find_ref_obj_id_regno doesn't catch this since we pass in false for err_multi) > /* Check that BTF function arguments match actual types that the > * verifier sees. > */ > for (i = 0; i < nargs; i++) { > - struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[i + 1]; > const struct btf_type *t, *ref_t, *resolve_ret; > enum bpf_arg_type arg_type = ARG_DONTCARE; > u32 regno = i + 1, ref_id, type_size; > bool is_ret_buf_sz = false; > + struct bpf_reg_state *reg; > int kf_arg_type; > > + reg = ®s[regno]; > t = btf_type_skip_modifiers(btf, args[i].type, NULL); > > if (is_kfunc_arg_ignore(btf, &args[i])) > @@ -9528,18 +9632,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EACCES; > } > > - if (reg->ref_obj_id) { > - if (is_kfunc_release(meta) && meta->ref_obj_id) { > - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", > - regno, reg->ref_obj_id, > - meta->ref_obj_id); > - return -EFAULT; > - } > - meta->ref_obj_id = reg->ref_obj_id; > - if (is_kfunc_release(meta)) > - meta->release_regno = regno; > - } > - > ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); > ref_tname = btf_name_by_offset(btf, ref_t->name_off); > > @@ -9585,7 +9677,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > return -EFAULT; > } > > - if (is_kfunc_release(meta) && reg->ref_obj_id) > + if (is_kfunc_release(meta) && regno == meta->release_regno) I don't think we need the "is_kfunc_release(meta)" check here since meta->release_regno is set to a regno only when is_kfunc_release(meta) is true > arg_type |= OBJ_RELEASE; > ret = check_func_arg_reg_off(env, reg, regno, arg_type); > if (ret < 0) > @@ -9747,12 +9839,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ > } > } > > - if (is_kfunc_release(meta) && !meta->release_regno) { > - verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", > - func_name); > - return -EINVAL; > - } > - > return 0; > } > > -- > 2.30.2 >
On 2/14/23 4:13 PM, Joanne Koong wrote: > On Tue, Feb 14, 2023 at 11:17 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >> >> Currently the ref_obj_id and OBJ_RELEASE searching is done in the code >> that examines each individual arg (check_func_arg for helpers and >> check_kfunc_args inner loop for kfuncs). This patch pulls out this >> searching to occur before individual arg type handling, resulting in a >> cleaner separation of logic and shared logic between kfuncs and helpers. >> >> The logic for this searching is already very similar between kfuncs and >> helpers: >> >> Kfuncs: >> * Function-level KF_RELEASE flag indicates that the kfunc releases >> some previously-acquired arg >> * Verifier searches through arg regs to find those with ref_obj_id set >> * One such arg reg is selected. If multiple arg regs have ref_obj_id >> set, the last one (by regno) is chosen to be released >> >> Helpers: >> * OBJ_RELEASE is used in function proto to tag a particular arg as the >> one that should be released >> * There can only be one such tagged arg >> * Verifier confirms that only one arg reg has ref_obj_id set and that >> that reg matches expected OBJ_RELEASE arg >> * If OBJ_RELEASE arg doesn't have a matching ref_obj_id arg reg, or >> some other arg reg has ref_obj_id, it's an invalid state >> >> It's a long-term goal to merge as much kfunc and helper logic as >> possible. Merging the similar functionality here is a small step in that >> direction. >> >> Two new helper functions are added: >> * args_find_ref_obj_id_regno >> * For helpers and kfuncs. Searches through arg regs to find >> ref_obj_id reg and returns its regno. >> >> * helper_proto_find_release_arg_regno >> * For helpers only. Searches through fn proto args to find the >> OBJ_RELEASE arg and returns the corresponding regno. >> >> The refactoring strives to keep failure logic and error messages >> unchanged. However, because the release arg searching is now done before >> any arg-specific type checking, verifier states that are invalid due to >> both invalid release arg state _and_ some type- or helper-specific >> checking logic might see the release arg-related error message first, >> when previously verification would fail for the other reason. >> >> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >> --- >> v2 -> v3: >> * Edit patch summary for clarity >> * Correct err_multi comment in args_find_ref_obj_id_regno doc string >> * Rebase onto latest bpf-next: 'Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"' >> >> v1 -> v2: https://lore.kernel.org/bpf/20230121002417.1684602-1-davemarchevsky@fb.com/ >> * Fix uninitialized variable complaint (kernel test bot) >> * Add err_multi param to args_find_ref_obj_id_regno - kfunc arg reg >> checking wasn't erroring if multiple ref_obj_id arg regs were found, >> retain this behavior >> >> v0 -> v1: https://lore.kernel.org/bpf/20221217082506.1570898-2-davemarchevsky@fb.com/ >> * Remove allow_multi from args_find_ref_obj_id_regno, no need to >> support multiple ref_obj_id arg regs >> * No need to use temp variable 'i' to count nargs (David) >> * Proper formatting of function-level comments on newly-added helpers (David) >> >> kernel/bpf/verifier.c | 220 +++++++++++++++++++++++++++++------------- >> 1 file changed, 153 insertions(+), 67 deletions(-) >> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 21e08c111702..c0d01085f44f 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -6735,48 +6735,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, >> return err; >> > [...] >> static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn, >> int *insn_idx_p) >> { >> enum bpf_prog_type prog_type = resolve_prog_type(env->prog); >> + int i, err, func_id, nargs, release_regno, ref_regno; >> const struct bpf_func_proto *fn = NULL; >> enum bpf_return_type ret_type; >> enum bpf_type_flag ret_flag; >> @@ -8115,7 +8178,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn >> struct bpf_call_arg_meta meta; >> int insn_idx = *insn_idx_p; >> bool changes_data; >> - int i, err, func_id; >> >> /* find function prototype */ >> func_id = insn->imm; >> @@ -8179,8 +8241,37 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn >> } >> >> meta.func_id = func_id; >> + regs = cur_regs(env); >> + >> + /* find actual arg count */ >> + for (nargs = 0; nargs < MAX_BPF_FUNC_REG_ARGS; nargs++) >> + if (fn->arg_type[nargs] == ARG_DONTCARE) >> + break; >> + >> + release_regno = helper_proto_find_release_arg_regno(env, fn, nargs); >> + if (release_regno < 0) >> + return release_regno; >> + >> + ref_regno = args_find_ref_obj_id_regno(env, regs, nargs, true); >> + if (ref_regno < 0) >> + return ref_regno; >> + else if (ref_regno > 0) >> + meta.ref_obj_id = regs[ref_regno].ref_obj_id; > > nit: I think it's easier to read if this ref_regno logic gets moved > below the release_regno logic, so that all the release_regno logic is > together > Sorry for resurrecting this old thread. I sent a v4 which applies feedback from this message, except for this nit and one other suggestion (addressed below). I agree with this readability nit, but did not apply the suggestion as this weird order is intentional in order to match the pre-refactoring order of error checks as much as possible. >> + >> + if (release_regno > 0) { >> + if (!regs[release_regno].ref_obj_id && >> + !register_is_null(®s[release_regno]) && >> + !arg_type_is_dynptr(fn->arg_type[release_regno - BPF_REG_1])) { >> + verbose(env, "R%d must be referenced when passed to release function\n", >> + release_regno); >> + return -EINVAL; >> + } >> + >> + meta.release_regno = release_regno; >> + } >> + >> /* check args */ >> - for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { >> + for (i = 0; i < nargs; i++) { >> err = check_func_arg(env, i, &meta, fn); >> if (err) >> return err; >> @@ -8204,8 +8295,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn >> return err; >> } >> >> - regs = cur_regs(env); >> - >> /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot >> * be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr >> * is safe to do directly. >> @@ -9442,10 +9531,11 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env, >> static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta) >> { >> const char *func_name = meta->func_name, *ref_tname; >> + struct bpf_reg_state *regs = cur_regs(env); >> const struct btf *btf = meta->btf; >> const struct btf_param *args; >> + int ret, ref_regno; >> u32 i, nargs; >> - int ret; >> >> args = (const struct btf_param *)(meta->func_proto + 1); >> nargs = btf_type_vlen(meta->func_proto); >> @@ -9455,17 +9545,31 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ >> return -EINVAL; >> } >> >> + ref_regno = args_find_ref_obj_id_regno(env, cur_regs(env), nargs, false); > > nit: I think we can just pass in "regs" as the 2nd arg > >> + if (ref_regno < 0) { >> + return ref_regno; >> + } else if (!ref_regno && is_kfunc_release(meta)) { >> + verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", >> + func_name); >> + return -EINVAL; >> + } >> + >> + meta->ref_obj_id = regs[ref_regno].ref_obj_id; >> + if (is_kfunc_release(meta)) >> + meta->release_regno = ref_regno; >> + > > I think we also need to check that if the kfunc is a release func then > there can't be more than one arg reg with a set ref_obj_id (the > earlier call to args_find_ref_obj_id_regno doesn't catch this since we > pass in false for err_multi) > This "if more than one arg reg w/ a set ref_obj_id, consider last such arg reg to be ref_regno" logic is preserving existing behavior. But more generally, RELEASE is really a parameter-level thing: if the function is RELEASEing some passed-in resource, that parameter should be tagged as such (OBJ_RELEASE type flag in helper func proto does this more cleanly IMO), then there's no need to rely on heuristics to guess what's being released. Might as well preserve the pre-refacotring heuristic here and move to parameter-level flags for kfuncs eventually. >> /* Check that BTF function arguments match actual types that the >> * verifier sees. >> */ >> for (i = 0; i < nargs; i++) { >> - struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[i + 1]; >> const struct btf_type *t, *ref_t, *resolve_ret; >> enum bpf_arg_type arg_type = ARG_DONTCARE; >> u32 regno = i + 1, ref_id, type_size; >> bool is_ret_buf_sz = false; >> + struct bpf_reg_state *reg; >> int kf_arg_type; >> >> + reg = ®s[regno]; >> t = btf_type_skip_modifiers(btf, args[i].type, NULL); >> >> if (is_kfunc_arg_ignore(btf, &args[i])) >> @@ -9528,18 +9632,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ >> return -EACCES; >> } >> >> - if (reg->ref_obj_id) { >> - if (is_kfunc_release(meta) && meta->ref_obj_id) { >> - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", >> - regno, reg->ref_obj_id, >> - meta->ref_obj_id); >> - return -EFAULT; >> - } >> - meta->ref_obj_id = reg->ref_obj_id; >> - if (is_kfunc_release(meta)) >> - meta->release_regno = regno; >> - } >> - >> ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); >> ref_tname = btf_name_by_offset(btf, ref_t->name_off); >> >> @@ -9585,7 +9677,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ >> return -EFAULT; >> } >> >> - if (is_kfunc_release(meta) && reg->ref_obj_id) >> + if (is_kfunc_release(meta) && regno == meta->release_regno) > > I don't think we need the "is_kfunc_release(meta)" check here since > meta->release_regno is set to a regno only when is_kfunc_release(meta) > is true > >> arg_type |= OBJ_RELEASE; >> ret = check_func_arg_reg_off(env, reg, regno, arg_type); >> if (ret < 0) >> @@ -9747,12 +9839,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ >> } >> } >> >> - if (is_kfunc_release(meta) && !meta->release_regno) { >> - verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", >> - func_name); >> - return -EINVAL; >> - } >> - >> return 0; >> } >> >> -- >> 2.30.2 >>
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 21e08c111702..c0d01085f44f 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6735,48 +6735,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, return err; skip_type_check: - if (arg_type_is_release(arg_type)) { - if (arg_type_is_dynptr(arg_type)) { - struct bpf_func_state *state = func(env, reg); - int spi; - - /* Only dynptr created on stack can be released, thus - * the get_spi and stack state checks for spilled_ptr - * should only be done before process_dynptr_func for - * PTR_TO_STACK. - */ - if (reg->type == PTR_TO_STACK) { - spi = dynptr_get_spi(env, reg); - if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) { - verbose(env, "arg %d is an unacquired reference\n", regno); - return -EINVAL; - } - } else { - verbose(env, "cannot release unowned const bpf_dynptr\n"); - return -EINVAL; - } - } else if (!reg->ref_obj_id && !register_is_null(reg)) { - verbose(env, "R%d must be referenced when passed to release function\n", - regno); - return -EINVAL; - } - if (meta->release_regno) { - verbose(env, "verifier internal error: more than one release argument\n"); - return -EFAULT; - } - meta->release_regno = regno; - } - - if (reg->ref_obj_id) { - if (meta->ref_obj_id) { - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", - regno, reg->ref_obj_id, - meta->ref_obj_id); - return -EFAULT; - } - meta->ref_obj_id = reg->ref_obj_id; - } - switch (base_type(arg_type)) { case ARG_CONST_MAP_PTR: /* bpf_map_xxx(map_ptr) call: remember that map_ptr */ @@ -6891,6 +6849,26 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, err = check_mem_size_reg(env, reg, regno, true, meta); break; case ARG_PTR_TO_DYNPTR: + if (meta->release_regno == regno) { + struct bpf_func_state *state = func(env, reg); + int spi; + + /* Only dynptr created on stack can be released, thus + * the get_spi and stack state checks for spilled_ptr + * should only be done before process_dynptr_func for + * PTR_TO_STACK. + */ + if (reg->type == PTR_TO_STACK) { + spi = dynptr_get_spi(env, reg); + if (spi < 0 || !state->stack[spi].spilled_ptr.ref_obj_id) { + verbose(env, "arg %d is an unacquired reference\n", regno); + return -EINVAL; + } + } else { + verbose(env, "cannot release unowned const bpf_dynptr\n"); + return -EINVAL; + } + } err = process_dynptr_func(env, regno, arg_type, meta); if (err) return err; @@ -8104,10 +8082,95 @@ static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno state->callback_subprogno == subprogno); } +/** + * args_find_ref_obj_id_regno() - Find regno that should become meta->ref_obj_id + * @env: Verifier env + * @regs: Regs to search for ref_obj_id + * @nargs: Number of arg regs to search + * @err_multi: Should this function error if multiple ref_obj_id args found + * + * Call arg meta's ref_obj_id is used to either: + * * For release funcs, keep track of ref that needs to be released + * * For other funcs, keep track of ref that needs to be propagated to retval + * + * Find the arg regno with nonzero ref_obj_id + * + * If err_multi is false and multiple ref_obj_id arg regs are seen, regno of the + * last one is returned + * + * Return: + * * On success, regno that should become meta->ref_obj_id (regno > 0 since + * BPF_REG_1 is first arg + * * 0 if no arg had ref_obj_id set + * * -err if some invalid arg reg state + */ +static int args_find_ref_obj_id_regno(struct bpf_verifier_env *env, struct bpf_reg_state *regs, + u32 nargs, bool err_multi) +{ + struct bpf_reg_state *reg; + u32 i, regno, found_regno = 0; + + for (i = 0; i < nargs; i++) { + regno = i + BPF_REG_1; + reg = ®s[regno]; + + if (!reg->ref_obj_id) + continue; + + if (found_regno && err_multi) { + verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", + regno, reg->ref_obj_id, regs[found_regno].ref_obj_id); + return -EFAULT; + } + + found_regno = regno; + } + + return found_regno; +} + +/** + * helper_proto_find_release_arg_regno() - Find OBJ_RELEASE arg in func proto + * @env: Verifier env + * @fn: Func proto to search for OBJ_RELEASE + * @nargs: Number of arg specs to search + * + * For helpers, to determine which arg reg should be released, loop through + * func proto arg specification to find arg with OBJ_RELEASE + * + * Return: + * * On success, regno of single OBJ_RELEASE arg + * * 0 if no arg in the proto was OBJ_RELEASE + * * -err if some invalid func proto state + */ +static int helper_proto_find_release_arg_regno(struct bpf_verifier_env *env, + const struct bpf_func_proto *fn, u32 nargs) +{ + enum bpf_arg_type arg_type; + int i, release_regno = 0; + + for (i = 0; i < nargs; i++) { + arg_type = fn->arg_type[i]; + + if (!arg_type_is_release(arg_type)) + continue; + + if (release_regno) { + verbose(env, "verifier internal error: more than one release argument\n"); + return -EFAULT; + } + + release_regno = i + BPF_REG_1; + } + + return release_regno; +} + static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn, int *insn_idx_p) { enum bpf_prog_type prog_type = resolve_prog_type(env->prog); + int i, err, func_id, nargs, release_regno, ref_regno; const struct bpf_func_proto *fn = NULL; enum bpf_return_type ret_type; enum bpf_type_flag ret_flag; @@ -8115,7 +8178,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn struct bpf_call_arg_meta meta; int insn_idx = *insn_idx_p; bool changes_data; - int i, err, func_id; /* find function prototype */ func_id = insn->imm; @@ -8179,8 +8241,37 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn } meta.func_id = func_id; + regs = cur_regs(env); + + /* find actual arg count */ + for (nargs = 0; nargs < MAX_BPF_FUNC_REG_ARGS; nargs++) + if (fn->arg_type[nargs] == ARG_DONTCARE) + break; + + release_regno = helper_proto_find_release_arg_regno(env, fn, nargs); + if (release_regno < 0) + return release_regno; + + ref_regno = args_find_ref_obj_id_regno(env, regs, nargs, true); + if (ref_regno < 0) + return ref_regno; + else if (ref_regno > 0) + meta.ref_obj_id = regs[ref_regno].ref_obj_id; + + if (release_regno > 0) { + if (!regs[release_regno].ref_obj_id && + !register_is_null(®s[release_regno]) && + !arg_type_is_dynptr(fn->arg_type[release_regno - BPF_REG_1])) { + verbose(env, "R%d must be referenced when passed to release function\n", + release_regno); + return -EINVAL; + } + + meta.release_regno = release_regno; + } + /* check args */ - for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) { + for (i = 0; i < nargs; i++) { err = check_func_arg(env, i, &meta, fn); if (err) return err; @@ -8204,8 +8295,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn return err; } - regs = cur_regs(env); - /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot * be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr * is safe to do directly. @@ -9442,10 +9531,11 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env, static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta) { const char *func_name = meta->func_name, *ref_tname; + struct bpf_reg_state *regs = cur_regs(env); const struct btf *btf = meta->btf; const struct btf_param *args; + int ret, ref_regno; u32 i, nargs; - int ret; args = (const struct btf_param *)(meta->func_proto + 1); nargs = btf_type_vlen(meta->func_proto); @@ -9455,17 +9545,31 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ return -EINVAL; } + ref_regno = args_find_ref_obj_id_regno(env, cur_regs(env), nargs, false); + if (ref_regno < 0) { + return ref_regno; + } else if (!ref_regno && is_kfunc_release(meta)) { + verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", + func_name); + return -EINVAL; + } + + meta->ref_obj_id = regs[ref_regno].ref_obj_id; + if (is_kfunc_release(meta)) + meta->release_regno = ref_regno; + /* Check that BTF function arguments match actual types that the * verifier sees. */ for (i = 0; i < nargs; i++) { - struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[i + 1]; const struct btf_type *t, *ref_t, *resolve_ret; enum bpf_arg_type arg_type = ARG_DONTCARE; u32 regno = i + 1, ref_id, type_size; bool is_ret_buf_sz = false; + struct bpf_reg_state *reg; int kf_arg_type; + reg = ®s[regno]; t = btf_type_skip_modifiers(btf, args[i].type, NULL); if (is_kfunc_arg_ignore(btf, &args[i])) @@ -9528,18 +9632,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ return -EACCES; } - if (reg->ref_obj_id) { - if (is_kfunc_release(meta) && meta->ref_obj_id) { - verbose(env, "verifier internal error: more than one arg with ref_obj_id R%d %u %u\n", - regno, reg->ref_obj_id, - meta->ref_obj_id); - return -EFAULT; - } - meta->ref_obj_id = reg->ref_obj_id; - if (is_kfunc_release(meta)) - meta->release_regno = regno; - } - ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); ref_tname = btf_name_by_offset(btf, ref_t->name_off); @@ -9585,7 +9677,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ return -EFAULT; } - if (is_kfunc_release(meta) && reg->ref_obj_id) + if (is_kfunc_release(meta) && regno == meta->release_regno) arg_type |= OBJ_RELEASE; ret = check_func_arg_reg_off(env, reg, regno, arg_type); if (ret < 0) @@ -9747,12 +9839,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ } } - if (is_kfunc_release(meta) && !meta->release_regno) { - verbose(env, "release kernel function %s expects refcounted PTR_TO_BTF_ID\n", - func_name); - return -EINVAL; - } - return 0; }
Currently the ref_obj_id and OBJ_RELEASE searching is done in the code that examines each individual arg (check_func_arg for helpers and check_kfunc_args inner loop for kfuncs). This patch pulls out this searching to occur before individual arg type handling, resulting in a cleaner separation of logic and shared logic between kfuncs and helpers. The logic for this searching is already very similar between kfuncs and helpers: Kfuncs: * Function-level KF_RELEASE flag indicates that the kfunc releases some previously-acquired arg * Verifier searches through arg regs to find those with ref_obj_id set * One such arg reg is selected. If multiple arg regs have ref_obj_id set, the last one (by regno) is chosen to be released Helpers: * OBJ_RELEASE is used in function proto to tag a particular arg as the one that should be released * There can only be one such tagged arg * Verifier confirms that only one arg reg has ref_obj_id set and that that reg matches expected OBJ_RELEASE arg * If OBJ_RELEASE arg doesn't have a matching ref_obj_id arg reg, or some other arg reg has ref_obj_id, it's an invalid state It's a long-term goal to merge as much kfunc and helper logic as possible. Merging the similar functionality here is a small step in that direction. Two new helper functions are added: * args_find_ref_obj_id_regno * For helpers and kfuncs. Searches through arg regs to find ref_obj_id reg and returns its regno. * helper_proto_find_release_arg_regno * For helpers only. Searches through fn proto args to find the OBJ_RELEASE arg and returns the corresponding regno. The refactoring strives to keep failure logic and error messages unchanged. However, because the release arg searching is now done before any arg-specific type checking, verifier states that are invalid due to both invalid release arg state _and_ some type- or helper-specific checking logic might see the release arg-related error message first, when previously verification would fail for the other reason. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- v2 -> v3: * Edit patch summary for clarity * Correct err_multi comment in args_find_ref_obj_id_regno doc string * Rebase onto latest bpf-next: 'Revert "bpf: Add --skip_encoding_btf_inconsistent_proto, --btf_gen_optimized to pahole flags for v1.25"' v1 -> v2: https://lore.kernel.org/bpf/20230121002417.1684602-1-davemarchevsky@fb.com/ * Fix uninitialized variable complaint (kernel test bot) * Add err_multi param to args_find_ref_obj_id_regno - kfunc arg reg checking wasn't erroring if multiple ref_obj_id arg regs were found, retain this behavior v0 -> v1: https://lore.kernel.org/bpf/20221217082506.1570898-2-davemarchevsky@fb.com/ * Remove allow_multi from args_find_ref_obj_id_regno, no need to support multiple ref_obj_id arg regs * No need to use temp variable 'i' to count nargs (David) * Proper formatting of function-level comments on newly-added helpers (David) kernel/bpf/verifier.c | 220 +++++++++++++++++++++++++++++------------- 1 file changed, 153 insertions(+), 67 deletions(-)