Fixed-size memory arguments went through two paths: helpers called check_helper_mem_access() directly, while kfuncs and global subprogs used check_mem_reg(). Route the helper MEM_FIXED_SIZE case through check_mem_reg() too so all three share the same check. This also fixes a bug in the helper path. When passing a NULL to PTR_MAYBE_NULL | ARG_PTR_TO_FIXED_SIZE_MEM argument, the program would be falsely rejected by check_helper_mem_access(). This is not triggerable since there is no such kind of helper. Also, note that check_reg_type() still make sure NULL cannot be passed to an argument not marked with PTR_MAYBE_NULL. Signed-off-by: Amery Hung --- kernel/bpf/verifier.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6520742d51b5..be5d93c0da81 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -6941,11 +6941,14 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, } static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, - argno_t argno, u32 mem_size, struct bpf_call_arg_meta *meta) + argno_t argno, u32 mem_size, enum bpf_access_type access_type, + struct bpf_call_arg_meta *meta) { + /* meta->btf is set only for kfuncs; a NULL meta means a global subprog. */ + bool helper = meta && !meta->btf; bool may_be_null = type_may_be_null(reg->type); struct bpf_reg_state saved_reg; - int err; + int size, err = 0; if (bpf_register_is_null(reg)) return 0; @@ -6965,10 +6968,12 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg mark_ptr_not_null_reg(reg); } - int size = base_type(reg->type) == PTR_TO_STACK ? -(int)mem_size : mem_size; + size = (!helper && base_type(reg->type) == PTR_TO_STACK) ? -(int)mem_size : mem_size; - err = check_helper_mem_access(env, reg, argno, size, BPF_READ, true, meta); - err = err ?: check_helper_mem_access(env, reg, argno, size, BPF_WRITE, true, meta); + if (access_type & BPF_READ) + err = check_helper_mem_access(env, reg, argno, size, BPF_READ, true, meta); + if (!err && (access_type & BPF_WRITE)) + err = check_helper_mem_access(env, reg, argno, size, BPF_WRITE, true, meta); if (may_be_null) *reg = saved_reg; @@ -8483,9 +8488,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, * next is_mem_size argument below. */ if (arg_type & MEM_FIXED_SIZE) { - err = check_helper_mem_access(env, reg, argno, fn->arg_size[arg], - arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, - false, meta); + err = check_mem_reg(env, reg, argno_from_reg(regno), fn->arg_size[arg], + arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ, meta); if (err) return err; if (arg_type & MEM_ALIGNED) @@ -9257,7 +9261,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog, ret = check_func_arg_reg_off(env, reg, argno, ARG_DONTCARE); if (ret < 0) return ret; - if (check_mem_reg(env, reg, argno, arg->mem_size, NULL)) + if (check_mem_reg(env, reg, argno, arg->mem_size, BPF_READ | BPF_WRITE, NULL)) return -EINVAL; if (!(arg->arg_type & PTR_MAYBE_NULL) && (reg->type & PTR_MAYBE_NULL)) { bpf_log(log, "%s is expected to be non-NULL\n", @@ -12403,7 +12407,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me ref_tname, PTR_ERR(resolve_ret)); return -EINVAL; } - ret = check_mem_reg(env, reg, argno, type_size, meta); + ret = check_mem_reg(env, reg, argno, type_size, BPF_READ | BPF_WRITE, meta); if (ret < 0) return ret; break; -- 2.52.0