Skip to content

Commit

Permalink
bpf: move {prev_,}insn_idx into verifier env
Browse files Browse the repository at this point in the history 
Move prev_insn_idx and insn_idx from the do_check() function into
the verifier environment, so they can be read inside the various
helper functions for handling the instructions. It's easier to put
this into the environment rather than changing all call-sites only
to pass it along. insn_idx is useful in particular since this later
on allows to hold state in env->insn_aux_data[env->insn_idx].

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Daniel Borkmann authored and Alexei Starovoitov committed Jan 3, 2019
1 parent 8b6b25c commit c08435e
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 38 deletions.
2 changes: 2 additions & 0 deletions include/linux/bpf_verifier.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ struct bpf_subprog_info {
* one verifier_env per bpf_check() call
*/
struct bpf_verifier_env {
u32 insn_idx;
u32 prev_insn_idx;
struct bpf_prog *prog; /* eBPF program being verified */
const struct bpf_verifier_ops *ops;
struct bpf_verifier_stack_elem *head; /* stack of verifier states to be processed */
Expand Down
76 changes: 38 additions & 38 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5650,7 +5650,6 @@ static int do_check(struct bpf_verifier_env *env)
struct bpf_insn *insns = env->prog->insnsi;
struct bpf_reg_state *regs;
int insn_cnt = env->prog->len, i;
int insn_idx, prev_insn_idx = 0;
int insn_processed = 0;
bool do_print_state = false;

Expand All @@ -5670,19 +5669,19 @@ static int do_check(struct bpf_verifier_env *env)
BPF_MAIN_FUNC /* callsite */,
0 /* frameno */,
0 /* subprogno, zero == main subprog */);
insn_idx = 0;

for (;;) {
struct bpf_insn *insn;
u8 class;
int err;

if (insn_idx >= insn_cnt) {
if (env->insn_idx >= insn_cnt) {
verbose(env, "invalid insn idx %d insn_cnt %d\n",
insn_idx, insn_cnt);
env->insn_idx, insn_cnt);
return -EFAULT;
}

insn = &insns[insn_idx];
insn = &insns[env->insn_idx];
class = BPF_CLASS(insn->code);

if (++insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) {
Expand All @@ -5692,17 +5691,17 @@ static int do_check(struct bpf_verifier_env *env)
return -E2BIG;
}

err = is_state_visited(env, insn_idx);
err = is_state_visited(env, env->insn_idx);
if (err < 0)
return err;
if (err == 1) {
/* found equivalent state, can prune the search */
if (env->log.level) {
if (do_print_state)
verbose(env, "\nfrom %d to %d: safe\n",
prev_insn_idx, insn_idx);
env->prev_insn_idx, env->insn_idx);
else
verbose(env, "%d: safe\n", insn_idx);
verbose(env, "%d: safe\n", env->insn_idx);
}
goto process_bpf_exit;
}
Expand All @@ -5715,10 +5714,10 @@ static int do_check(struct bpf_verifier_env *env)

if (env->log.level > 1 || (env->log.level && do_print_state)) {
if (env->log.level > 1)
verbose(env, "%d:", insn_idx);
verbose(env, "%d:", env->insn_idx);
else
verbose(env, "\nfrom %d to %d:",
prev_insn_idx, insn_idx);
env->prev_insn_idx, env->insn_idx);
print_verifier_state(env, state->frame[state->curframe]);
do_print_state = false;
}
Expand All @@ -5729,20 +5728,20 @@ static int do_check(struct bpf_verifier_env *env)
.private_data = env,
};

verbose_linfo(env, insn_idx, "; ");
verbose(env, "%d: ", insn_idx);
verbose_linfo(env, env->insn_idx, "; ");
verbose(env, "%d: ", env->insn_idx);
print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);
}

if (bpf_prog_is_dev_bound(env->prog->aux)) {
err = bpf_prog_offload_verify_insn(env, insn_idx,
prev_insn_idx);
err = bpf_prog_offload_verify_insn(env, env->insn_idx,
env->prev_insn_idx);
if (err)
return err;
}

regs = cur_regs(env);
env->insn_aux_data[insn_idx].seen = true;
env->insn_aux_data[env->insn_idx].seen = true;

if (class == BPF_ALU || class == BPF_ALU64) {
err = check_alu_op(env, insn);
Expand All @@ -5768,13 +5767,13 @@ static int do_check(struct bpf_verifier_env *env)
/* check that memory (src_reg + off) is readable,
* the state of dst_reg will be updated by this func
*/
err = check_mem_access(env, insn_idx, insn->src_reg, insn->off,
BPF_SIZE(insn->code), BPF_READ,
insn->dst_reg, false);
err = check_mem_access(env, env->insn_idx, insn->src_reg,
insn->off, BPF_SIZE(insn->code),
BPF_READ, insn->dst_reg, false);
if (err)
return err;

prev_src_type = &env->insn_aux_data[insn_idx].ptr_type;
prev_src_type = &env->insn_aux_data[env->insn_idx].ptr_type;

if (*prev_src_type == NOT_INIT) {
/* saw a valid insn
Expand All @@ -5799,10 +5798,10 @@ static int do_check(struct bpf_verifier_env *env)
enum bpf_reg_type *prev_dst_type, dst_reg_type;

if (BPF_MODE(insn->code) == BPF_XADD) {
err = check_xadd(env, insn_idx, insn);
err = check_xadd(env, env->insn_idx, insn);
if (err)
return err;
insn_idx++;
env->insn_idx++;
continue;
}

Expand All @@ -5818,13 +5817,13 @@ static int do_check(struct bpf_verifier_env *env)
dst_reg_type = regs[insn->dst_reg].type;

/* check that memory (dst_reg + off) is writeable */
err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
BPF_SIZE(insn->code), BPF_WRITE,
insn->src_reg, false);
err = check_mem_access(env, env->insn_idx, insn->dst_reg,
insn->off, BPF_SIZE(insn->code),
BPF_WRITE, insn->src_reg, false);
if (err)
return err;

prev_dst_type = &env->insn_aux_data[insn_idx].ptr_type;
prev_dst_type = &env->insn_aux_data[env->insn_idx].ptr_type;

if (*prev_dst_type == NOT_INIT) {
*prev_dst_type = dst_reg_type;
Expand Down Expand Up @@ -5852,9 +5851,9 @@ static int do_check(struct bpf_verifier_env *env)
}

/* check that memory (dst_reg + off) is writeable */
err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,
BPF_SIZE(insn->code), BPF_WRITE,
-1, false);
err = check_mem_access(env, env->insn_idx, insn->dst_reg,
insn->off, BPF_SIZE(insn->code),
BPF_WRITE, -1, false);
if (err)
return err;

Expand All @@ -5872,9 +5871,9 @@ static int do_check(struct bpf_verifier_env *env)
}

if (insn->src_reg == BPF_PSEUDO_CALL)
err = check_func_call(env, insn, &insn_idx);
err = check_func_call(env, insn, &env->insn_idx);
else
err = check_helper_call(env, insn->imm, insn_idx);
err = check_helper_call(env, insn->imm, env->insn_idx);
if (err)
return err;

Expand All @@ -5887,7 +5886,7 @@ static int do_check(struct bpf_verifier_env *env)
return -EINVAL;
}

insn_idx += insn->off + 1;
env->insn_idx += insn->off + 1;
continue;

} else if (opcode == BPF_EXIT) {
Expand All @@ -5901,8 +5900,8 @@ static int do_check(struct bpf_verifier_env *env)

if (state->curframe) {
/* exit from nested function */
prev_insn_idx = insn_idx;
err = prepare_func_exit(env, &insn_idx);
env->prev_insn_idx = env->insn_idx;
err = prepare_func_exit(env, &env->insn_idx);
if (err)
return err;
do_print_state = true;
Expand Down Expand Up @@ -5932,7 +5931,8 @@ static int do_check(struct bpf_verifier_env *env)
if (err)
return err;
process_bpf_exit:
err = pop_stack(env, &prev_insn_idx, &insn_idx);
err = pop_stack(env, &env->prev_insn_idx,
&env->insn_idx);
if (err < 0) {
if (err != -ENOENT)
return err;
Expand All @@ -5942,7 +5942,7 @@ static int do_check(struct bpf_verifier_env *env)
continue;
}
} else {
err = check_cond_jmp_op(env, insn, &insn_idx);
err = check_cond_jmp_op(env, insn, &env->insn_idx);
if (err)
return err;
}
Expand All @@ -5959,8 +5959,8 @@ static int do_check(struct bpf_verifier_env *env)
if (err)
return err;

insn_idx++;
env->insn_aux_data[insn_idx].seen = true;
env->insn_idx++;
env->insn_aux_data[env->insn_idx].seen = true;
} else {
verbose(env, "invalid BPF_LD mode\n");
return -EINVAL;
Expand All @@ -5970,7 +5970,7 @@ static int do_check(struct bpf_verifier_env *env)
return -EINVAL;
}

insn_idx++;
env->insn_idx++;
}

verbose(env, "processed %d insns (limit %d), stack depth ",
Expand Down

0 comments on commit c08435e

Please sign in to comment.