Skip to content

Commit

Permalink
bpf: Allow initializing dynptrs in kfuncs
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Joanne Koong authored and Alexei Starovoitov committed Mar 1, 2023
1 parent 7e0dac2 commit 1d18feb
Showing 1 changed file with 22 additions and 45 deletions.
67 changes: 22 additions & 45 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_*):
Expand All @@ -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");
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -6932,7 +6931,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 1d18feb

Please sign in to comment.