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>
This commit is contained in:
committed by
Alexei Starovoitov
parent
7e0dac2807
commit
1d18feb2c9
+22
-45
@@ -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 = ®s[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 = ®s[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, ®s[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
|
||||
|
||||
Reference in New Issue
Block a user