From: Alexei Starovoitov An extension replaces its target, a global function of another prog. From BTF, btf_prepare_func_args() derives the kind of each argument, such as the context, memory of a given size or a kernel object. The verifier checks the callers with the BTF of the target and the extension with its own BTF. Only btf_check_func_type_match() compares the two prototypes. For a pointer to a struct, it compares only the struct names and assumes that the pointer is the context. That was true until the commit below added pointers to memory. Later the __arg_* tags added more kinds. Thus an extension can rely on more than the callers guarantee (size, not NULL, kind). Then what the verifier proved for it is false at run time. Keep the result of btf_prepare_func_args() in bpf_func_info_aux and compare the two results at load and on attach to another target. Return -EINVAL unless the kind and the BTF id are the same and the memory is not larger. The extension can also accept NULL where the callers never pass it. An extension with the prototype and the tags of its target loads as before. An argument that is the context only because of __arg_ctx needs the tag in the extension too. The tag __arg_untrusted or __arg_arena on the extension only is safe, but it is rejected because the kind differs. Fixes: e5069b9c23b3 ("bpf: Support pointers in global func args") Signed-off-by: Alexei Starovoitov --- include/linux/bpf.h | 10 +++++++++ include/linux/bpf_verifier.h | 8 ------- kernel/bpf/btf.c | 10 ++++++--- kernel/bpf/verifier.c | 43 +++++++++++++++++++++++++++++++++++- 4 files changed, 59 insertions(+), 12 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 1a8c3e514284..0b44bcb4f56e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1752,12 +1752,22 @@ enum bpf_func_context { BPF_FUNC_CTX_CNT, }; +struct bpf_subprog_arg_info { + enum bpf_arg_type arg_type; + union { + u32 mem_size; + u32 btf_id; + }; +}; + struct bpf_func_info_aux { u16 linkage; bool unreliable; bool called[BPF_FUNC_CTX_CNT]; bool verified[BPF_FUNC_CTX_CNT]; bool called_with_pkt_ptrs; + /* what the callers of a global function are checked against */ + struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS]; }; enum bpf_jit_poke_reason { diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7baeb831281f..3cb91c4787c9 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -923,14 +923,6 @@ struct bpf_func_ptr { /* the subprog that a bpf_func_ptr pointed to was removed as dead code */ #define BPF_FUNC_PTR_DELETED ((u32)-1) -struct bpf_subprog_arg_info { - enum bpf_arg_type arg_type; - union { - u32 mem_size; - u32 btf_id; - }; -}; - enum priv_stack_mode { PRIV_STACK_UNKNOWN, NO_PRIV_STACK, diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index fe24e459c1b1..8bf39098ba97 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -8492,9 +8492,8 @@ static int btf_check_func_type_match(struct bpf_verifier_log *log, } /* This is an optional check to make program writing easier. * Compare names of structs and report an error to the user. - * btf_prepare_func_args() already checked that t2 struct - * is a context type. btf_prepare_func_args() will check - * later that t1 struct is a context type as well. + * check_ext_args() compares what btf_prepare_func_args() + * derives for the two arguments. */ s1 = btf_name_by_offset(btf1, t1->name_off); s2 = btf_name_by_offset(btf2, t2->name_off); @@ -9028,6 +9027,11 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog) return err; sub->arg_slot_cnt = slots_used; + /* check_ext_args() needs them after env is freed */ + if (is_global) + memcpy(subprog_aux(env, subprog)->args, sub->args, + sizeof(subprog_aux(env, subprog)->args)); + sub->args_cached = true; return 0; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a456adaa459b..1f19fbd627d2 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -22514,6 +22514,37 @@ static bool func_has_rcu_read_lock(const struct bpf_prog *prog, int subprog) !sub_aux->called[BPF_FUNC_CTX_NO_RCU_LOCK]; } +/* + * The callers of a global function are checked against the arguments that its + * BTF describes, and an extension is verified with those of its own BTF. It + * must not expect more than the callers of the function provide. + * + * While an extension is loaded, bpf_check_attach_target() runs before its + * arguments are known. check_attach_btf_id() does the check when they are. + */ +static int check_ext_args(struct bpf_verifier_log *log, const struct bpf_prog *prog, + const struct bpf_prog *tgt_prog, int subprog) +{ + const struct bpf_subprog_arg_info *tgt = tgt_prog->aux->func_info_aux[subprog].args; + const struct bpf_subprog_arg_info *ext = prog->aux->func_info_aux[0].args; + int i; + + for (i = 0; i < MAX_BPF_FUNC_REG_ARGS && ext[i].arg_type != ARG_UNUSED; i++) { + /* An extension may accept NULL where the callers never pass it. */ + if ((ext[i].arg_type == tgt[i].arg_type || + ext[i].arg_type == (tgt[i].arg_type | PTR_MAYBE_NULL)) && + (base_type(ext[i].arg_type) == ARG_PTR_TO_MEM ? + ext[i].mem_size <= tgt[i].mem_size : ext[i].btf_id == tgt[i].btf_id)) + continue; + + bpf_log(log, "Extension program's argument in R%d is incompatible with original\n", + i + 1); + return -EINVAL; + } + + return 0; +} + int bpf_check_attach_target(struct bpf_verifier_log *log, const struct bpf_prog *prog, const struct bpf_prog *tgt_prog, @@ -22801,7 +22832,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log, return -EINVAL; } if (prog_extension && - btf_check_type_match(log, prog, btf, t)) + (btf_check_type_match(log, prog, btf, t) || + check_ext_args(log, prog, tgt_prog, subprog))) return -EINVAL; t = btf_attach_func_proto(log, btf, btf_id); if (!t || !btf_type_is_func_proto(t)) @@ -23021,6 +23053,15 @@ static int check_attach_btf_id(struct bpf_verifier_env *env) */ env->ops = bpf_verifier_ops[tgt_prog->type]; prog->expected_attach_type = tgt_prog->expected_attach_type; + + /* only now it is known what the arguments of prog stand for */ + ret = btf_prepare_func_args(env, 0); + if (ret) + return ret; + ret = check_ext_args(&env->log, prog, tgt_prog, + find_tgt_subprog(tgt_prog, btf_id)); + if (ret) + return ret; } /* store info about the attachment target that will be used later */ -- 2.55.0