get_kfunc_ptr_arg_type() returned KF_ARG_PTR_TO_NULL when a nullable pointer argument was passed a NULL register. This folded a register-state decision (bpf_register_is_null()) into what is otherwise BTF-based argument classification, and it short-circuited before the BTF_ID/MEM resolution. Drop KF_ARG_PTR_TO_NULL and handle the NULL case in check_kfunc_args() instead: a nullable argument that is actually NULL is skipped. Note that it is okay to skip even when it is a mem+size pair because the size argument check has been moved to the scalar section. The skip is done before get_kfunc_ptr_arg_type() so that a NULL passed to a nullable non-scalar-struct argument is not newly rejected by the BTF_ID/MEM resolution. No functional change. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index f6b373630ca9..64668e7d184a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11082,7 +11082,6 @@ enum kfunc_ptr_arg_type { KF_ARG_PTR_TO_CALLBACK, KF_ARG_PTR_TO_RB_ROOT, KF_ARG_PTR_TO_RB_NODE, - KF_ARG_PTR_TO_NULL, KF_ARG_PTR_TO_CONST_STR, KF_ARG_CONST_MAP_PTR, KF_ARG_PTR_TO_TIMER, @@ -11359,11 +11358,6 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, meta->func_id == special_kfunc_list[KF_bpf_session_cookie]) return KF_ARG_PTR_TO_CTX; - if (arg + 1 < nargs && - (is_kfunc_arg_mem_size(meta->btf, &args[arg + 1]) || - is_kfunc_arg_const_mem_size(meta->btf, &args[arg + 1]))) - arg_mem_size = true; - /* In this function, we verify the kfunc's BTF as per the argument type, * leaving the rest of the verification with respect to the register * type to our caller. When a set of conditions hold in the BTF type of @@ -11372,10 +11366,6 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, if (btf_is_prog_ctx_type(&env->log, meta->btf, t, resolve_prog_type(env->prog), arg)) return KF_ARG_PTR_TO_CTX; - if (is_kfunc_arg_nullable(meta->btf, &args[arg]) && bpf_register_is_null(reg) && - !arg_mem_size) - return KF_ARG_PTR_TO_NULL; - if (is_kfunc_arg_alloc_obj(meta->btf, &args[arg])) return KF_ARG_PTR_TO_ALLOC_BTF_ID; @@ -11437,6 +11427,11 @@ get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, if (is_kfunc_arg_callback(env, meta->btf, &args[arg])) return KF_ARG_PTR_TO_CALLBACK; + if (arg + 1 < nargs && + (is_kfunc_arg_mem_size(meta->btf, &args[arg + 1]) || + is_kfunc_arg_const_mem_size(meta->btf, &args[arg + 1]))) + arg_mem_size = true; + /* This is the catch all argument type of register types supported by * check_helper_mem_access. However, we only allow when argument type is * pointer to scalar, or struct composed (recursively) of scalars. When @@ -12137,6 +12132,9 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id); ref_tname = btf_name_by_offset(btf, ref_t->name_off); + if (is_kfunc_arg_nullable(meta->btf, &args[i]) && bpf_register_is_null(reg)) + continue; + kf_arg_type = get_kfunc_ptr_arg_type(env, regs, meta, t, ref_t, ref_tname, args, i, nargs, argno, reg); if (kf_arg_type < 0) @@ -12149,8 +12147,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me } switch (kf_arg_type) { - case KF_ARG_PTR_TO_NULL: - continue; case KF_ARG_PTR_TO_ALLOC_BTF_ID: case KF_ARG_PTR_TO_BTF_ID: if (!is_trusted_reg(env, reg)) { @@ -12414,19 +12410,16 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me case KF_ARG_PTR_TO_MEM_SIZE: { struct bpf_reg_state *buff_reg = reg; - const struct btf_param *buff_arg = &args[i]; struct bpf_reg_state *size_reg = get_func_arg_reg(caller, regs, i + 1); argno_t next_argno = argno_from_arg(i + 2); - if (!bpf_register_is_null(buff_reg) || !is_kfunc_arg_nullable(meta->btf, buff_arg)) { - ret = check_mem_size_reg(env, buff_reg, size_reg, argno, next_argno, - BPF_READ | BPF_WRITE, true, meta); - if (ret < 0) { - verbose(env, "%s and ", reg_arg_name(env, argno)); - verbose(env, "%s memory, len pair leads to invalid memory access\n", - reg_arg_name(env, next_argno)); - return ret; - } + ret = check_mem_size_reg(env, buff_reg, size_reg, argno, next_argno, + BPF_READ | BPF_WRITE, true, meta); + if (ret < 0) { + verbose(env, "%s and ", reg_arg_name(env, argno)); + verbose(env, "%s memory, len pair leads to invalid memory access\n", + reg_arg_name(env, next_argno)); + return ret; } break; } -- 2.52.0