bpf: Allow initializing dynptrs in kfuncs

This change allows kfuncs to take in an uninitialized dynptr as a
parameter. Before this change, only helper functions could successfully
use uninitialized dynptrs. This change moves the memory access check
(including stack state growing and slot marking) into
process_dynptr_func(), which both helpers and kfuncs call into.

Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
Link: https://lore.kernel.org/r/20230301154953.641654-4-joannelkoong@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e0e00509846b6775ef5d00a8a4dd9319be145ec0..82e39fc5ed05dfd3e1e8b0617c87ca829d446567 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -268,7 +268,6 @@ struct bpf_call_arg_meta {
        u32 ret_btf_id;
        u32 subprogno;
        struct btf_field *kptr_field;
-       u8 uninit_dynptr_regno;
 };
 
 struct btf *btf_vmlinux;
@@ -6225,10 +6224,11 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
  * Helpers which do not mutate the bpf_dynptr set MEM_RDONLY in their argument
  * type, and declare it as 'const struct bpf_dynptr *' in their prototype.
  */
-static int process_dynptr_func(struct bpf_verifier_env *env, int regno,
-                              enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta)
+static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn_idx,
+                              enum bpf_arg_type arg_type)
 {
        struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
+       int err;
 
        /* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to an
         * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*):
@@ -6254,23 +6254,23 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno,
         *               to.
         */
        if (arg_type & MEM_UNINIT) {
+               int i;
+
                if (!is_dynptr_reg_valid_uninit(env, reg)) {
                        verbose(env, "Dynptr has to be an uninitialized dynptr\n");
                        return -EINVAL;
                }
 
-               /* We only support one dynptr being uninitialized at the moment,
-                * which is sufficient for the helper functions we have right now.
-                */
-               if (meta->uninit_dynptr_regno) {
-                       verbose(env, "verifier internal error: multiple uninitialized dynptr args\n");
-                       return -EFAULT;
+               /* we write BPF_DW bits (8 bytes) at a time */
+               for (i = 0; i < BPF_DYNPTR_SIZE; i += 8) {
+                       err = check_mem_access(env, insn_idx, regno,
+                                              i, BPF_DW, BPF_WRITE, -1, false);
+                       if (err)
+                               return err;
                }
 
-               meta->uninit_dynptr_regno = regno;
+               err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx);
        } else /* MEM_RDONLY and None case from above */ {
-               int err;
-
                /* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */
                if (reg->type == CONST_PTR_TO_DYNPTR && !(arg_type & MEM_RDONLY)) {
                        verbose(env, "cannot pass pointer to const bpf_dynptr, the helper mutates it\n");
@@ -6306,10 +6306,8 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno,
                }
 
                err = mark_dynptr_read(env, reg);
-               if (err)
-                       return err;
        }
-       return 0;
+       return err;
 }
 
 static bool arg_type_is_mem_size(enum bpf_arg_type type)
@@ -6719,7 +6717,8 @@ static int dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state
 
 static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
                          struct bpf_call_arg_meta *meta,
-                         const struct bpf_func_proto *fn)
+                         const struct bpf_func_proto *fn,
+                         int insn_idx)
 {
        u32 regno = BPF_REG_1 + arg;
        struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
@@ -6932,7 +6931,7 @@ skip_type_check:
                err = check_mem_size_reg(env, reg, regno, true, meta);
                break;
        case ARG_PTR_TO_DYNPTR:
-               err = process_dynptr_func(env, regno, arg_type, meta);
+               err = process_dynptr_func(env, regno, insn_idx, arg_type);
                if (err)
                        return err;
                break;
@@ -8218,7 +8217,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
        meta.func_id = func_id;
        /* check args */
        for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
-               err = check_func_arg(env, i, &meta, fn);
+               err = check_func_arg(env, i, &meta, fn, insn_idx);
                if (err)
                        return err;
        }
@@ -8243,30 +8242,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 
        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.
-        */
-       if (meta.uninit_dynptr_regno) {
-               if (regs[meta.uninit_dynptr_regno].type == CONST_PTR_TO_DYNPTR) {
-                       verbose(env, "verifier internal error: CONST_PTR_TO_DYNPTR cannot be initialized\n");
-                       return -EFAULT;
-               }
-               /* we write BPF_DW bits (8 bytes) at a time */
-               for (i = 0; i < BPF_DYNPTR_SIZE; i += 8) {
-                       err = check_mem_access(env, insn_idx, meta.uninit_dynptr_regno,
-                                              i, BPF_DW, BPF_WRITE, -1, false);
-                       if (err)
-                               return err;
-               }
-
-               err = mark_stack_slots_dynptr(env, &regs[meta.uninit_dynptr_regno],
-                                             fn->arg_type[meta.uninit_dynptr_regno - BPF_REG_1],
-                                             insn_idx);
-               if (err)
-                       return err;
-       }
-
        if (meta.release_regno) {
                err = -EINVAL;
                /* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot
@@ -9475,7 +9450,8 @@ static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
                                                  &meta->arg_rbtree_root.field);
 }
 
-static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta)
+static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta,
+                           int insn_idx)
 {
        const char *func_name = meta->func_name, *ref_tname;
        const struct btf *btf = meta->btf;
@@ -9672,7 +9648,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
                                return -EINVAL;
                        }
 
-                       ret = process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR | MEM_RDONLY, NULL);
+                       ret = process_dynptr_func(env, regno, insn_idx,
+                                                 ARG_PTR_TO_DYNPTR | MEM_RDONLY);
                        if (ret < 0)
                                return ret;
                        break;
@@ -9880,7 +9857,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
        }
 
        /* Check the arguments */
-       err = check_kfunc_args(env, &meta);
+       err = check_kfunc_args(env, &meta, insn_idx);
        if (err < 0)
                return err;
        /* In case of release function, we get register number of refcounted