bpf: move {prev_,}insn_idx into verifier env

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>
author: Daniel Borkmann <daniel@iogearbox.net> 2019-01-02 18:58:27 -0500
committer: Alexei Starovoitov <ast@kernel.org> 2019-01-02 19:01:24 -0500
commit: c08435ec7f2bc8f4109401f696fd55159b4b40cb (patch)
tree: 3e307a1a8967ae960b5110b5972dae885c843681 /kernel
parent: 8b6b25cf93b73e8e399adc55b0ffb9db32b881e0 (diff)
1 files changed, 38 insertions, 38 deletions
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 71d86e3024ae..afa8515bbb34 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -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;
@@ -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) {
@@ -5692,7 +5691,7 @@ 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) {
@@ -5700,9 +5699,9 @@ static int do_check(struct bpf_verifier_env *env)
                        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;
                }
@@ -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;
                }
@@ -5729,20 +5728,20 @@ static int do_check(struct bpf_verifier_env *env)
                                .private_data   = env,
                        };
-                        verbose_linfo(env, insn_idx, "; ");
+                        verbose_linfo(env, env->insn_idx, "; ");
-                        verbose(env, "%d: ", 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,
+                        err = bpf_prog_offload_verify_insn(env, env->insn_idx,
-                                                           prev_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);
@@ -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,
+                        err = check_mem_access(env, env->insn_idx, insn->src_reg,
-                                               BPF_SIZE(insn->code), BPF_READ,
+                                               insn->off, BPF_SIZE(insn->code),
-                                               insn->dst_reg, false);
+                                               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
@@ -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;
                        }
@@ -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,
+                        err = check_mem_access(env, env->insn_idx, insn->dst_reg,
-                                               BPF_SIZE(insn->code), BPF_WRITE,
+                                               insn->off, BPF_SIZE(insn->code),
-                                               insn->src_reg, false);
+                                               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;
@@ -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,
+                        err = check_mem_access(env, env->insn_idx, insn->dst_reg,
-                                               BPF_SIZE(insn->code), BPF_WRITE,
+                                               insn->off, BPF_SIZE(insn->code),
-                                               -1, false);
+                                               BPF_WRITE, -1, false);
                        if (err)
                                return err;
@@ -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;
@@ -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) {
@@ -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;
+                                        env->prev_insn_idx = env->insn_idx;
-                                        err = prepare_func_exit(env, &insn_idx);
+                                        err = prepare_func_exit(env, &env->insn_idx);
                                        if (err)
                                                return err;
                                        do_print_state = true;
@@ -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;
@@ -5942,7 +5942,7 @@ process_bpf_exit:
                                        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;
                        }
@@ -5959,8 +5959,8 @@ process_bpf_exit:
                                if (err)
                                        return err;
-                                insn_idx++;
+                                env->insn_idx++;
-                                env->insn_aux_data[insn_idx].seen = true;
+                                env->insn_aux_data[env->insn_idx].seen = true;
                        } else {
                                verbose(env, "invalid BPF_LD mode\n");
                                return -EINVAL;
@@ -5970,7 +5970,7 @@ process_bpf_exit:
                        return -EINVAL;
                }
-                insn_idx++;
+                env->insn_idx++;
        }
        verbose(env, "processed %d insns (limit %d), stack depth ",
author	Daniel Borkmann <daniel@iogearbox.net>	2019-01-02 18:58:27 -0500
committer	Alexei Starovoitov <ast@kernel.org>	2019-01-02 19:01:24 -0500
commit	c08435ec7f2bc8f4109401f696fd55159b4b40cb (patch)
tree	3e307a1a8967ae960b5110b5972dae885c843681 /kernel
parent	8b6b25cf93b73e8e399adc55b0ffb9db32b881e0 (diff)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 71d86e3024ae..afa8515bbb34 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c
@@ -5650,7 +5650,6 @@ static int do_check(struct bpf_verifier_env *env)
5650	struct bpf_insn *insns = env->prog->insnsi;	5650	struct bpf_insn *insns = env->prog->insnsi;
5651	struct bpf_reg_state *regs;	5651	struct bpf_reg_state *regs;
5652	int insn_cnt = env->prog->len, i;	5652	int insn_cnt = env->prog->len, i;
5653	int insn_idx, prev_insn_idx = 0;
5654	int insn_processed = 0;	5653	int insn_processed = 0;
5655	bool do_print_state = false;	5654	bool do_print_state = false;
5656		5655
@@ -5670,19 +5669,19 @@ static int do_check(struct bpf_verifier_env *env)
5670	BPF_MAIN_FUNC /* callsite */,	5669	BPF_MAIN_FUNC /* callsite */,
5671	0 /* frameno */,	5670	0 /* frameno */,
5672	0 /* subprogno, zero == main subprog */);	5671	0 /* subprogno, zero == main subprog */);
5673	insn_idx = 0;	5672
5674	for (;;) {	5673	for (;;) {
5675	struct bpf_insn *insn;	5674	struct bpf_insn *insn;
5676	u8 class;	5675	u8 class;
5677	int err;	5676	int err;
5678		5677
5679	if (insn_idx >= insn_cnt) {	5678	if (env->insn_idx >= insn_cnt) {
5680	verbose(env, "invalid insn idx %d insn_cnt %d\n",	5679	verbose(env, "invalid insn idx %d insn_cnt %d\n",
5681	insn_idx, insn_cnt);	5680	env->insn_idx, insn_cnt);
5682	return -EFAULT;	5681	return -EFAULT;
5683	}	5682	}
5684		5683
5685	insn = &insns[insn_idx];	5684	insn = &insns[env->insn_idx];
5686	class = BPF_CLASS(insn->code);	5685	class = BPF_CLASS(insn->code);
5687		5686
5688	if (++insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) {	5687	if (++insn_processed > BPF_COMPLEXITY_LIMIT_INSNS) {
@@ -5692,7 +5691,7 @@ static int do_check(struct bpf_verifier_env *env)
5692	return -E2BIG;	5691	return -E2BIG;
5693	}	5692	}
5694		5693
5695	err = is_state_visited(env, insn_idx);	5694	err = is_state_visited(env, env->insn_idx);
5696	if (err < 0)	5695	if (err < 0)
5697	return err;	5696	return err;
5698	if (err == 1) {	5697	if (err == 1) {
@@ -5700,9 +5699,9 @@ static int do_check(struct bpf_verifier_env *env)
5700	if (env->log.level) {	5699	if (env->log.level) {
5701	if (do_print_state)	5700	if (do_print_state)
5702	verbose(env, "\nfrom %d to %d: safe\n",	5701	verbose(env, "\nfrom %d to %d: safe\n",
5703	prev_insn_idx, insn_idx);	5702	env->prev_insn_idx, env->insn_idx);
5704	else	5703	else
5705	verbose(env, "%d: safe\n", insn_idx);	5704	verbose(env, "%d: safe\n", env->insn_idx);
5706	}	5705	}
5707	goto process_bpf_exit;	5706	goto process_bpf_exit;
5708	}	5707	}
@@ -5715,10 +5714,10 @@ static int do_check(struct bpf_verifier_env *env)
5715		5714
5716	if (env->log.level > 1 \|\| (env->log.level && do_print_state)) {	5715	if (env->log.level > 1 \|\| (env->log.level && do_print_state)) {
5717	if (env->log.level > 1)	5716	if (env->log.level > 1)
5718	verbose(env, "%d:", insn_idx);	5717	verbose(env, "%d:", env->insn_idx);
5719	else	5718	else
5720	verbose(env, "\nfrom %d to %d:",	5719	verbose(env, "\nfrom %d to %d:",
5721	prev_insn_idx, insn_idx);	5720	env->prev_insn_idx, env->insn_idx);
5722	print_verifier_state(env, state->frame[state->curframe]);	5721	print_verifier_state(env, state->frame[state->curframe]);
5723	do_print_state = false;	5722	do_print_state = false;
5724	}	5723	}
@@ -5729,20 +5728,20 @@ static int do_check(struct bpf_verifier_env *env)
5729	.private_data = env,	5728	.private_data = env,
5730	};	5729	};
5731		5730
5732	verbose_linfo(env, insn_idx, "; ");	5731	verbose_linfo(env, env->insn_idx, "; ");
5733	verbose(env, "%d: ", insn_idx);	5732	verbose(env, "%d: ", env->insn_idx);
5734	print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);	5733	print_bpf_insn(&cbs, insn, env->allow_ptr_leaks);
5735	}	5734	}
5736		5735
5737	if (bpf_prog_is_dev_bound(env->prog->aux)) {	5736	if (bpf_prog_is_dev_bound(env->prog->aux)) {
5738	err = bpf_prog_offload_verify_insn(env, insn_idx,	5737	err = bpf_prog_offload_verify_insn(env, env->insn_idx,
5739	prev_insn_idx);	5738	env->prev_insn_idx);
5740	if (err)	5739	if (err)
5741	return err;	5740	return err;
5742	}	5741	}
5743		5742
5744	regs = cur_regs(env);	5743	regs = cur_regs(env);
5745	env->insn_aux_data[insn_idx].seen = true;	5744	env->insn_aux_data[env->insn_idx].seen = true;
5746		5745
5747	if (class == BPF_ALU \|\| class == BPF_ALU64) {	5746	if (class == BPF_ALU \|\| class == BPF_ALU64) {
5748	err = check_alu_op(env, insn);	5747	err = check_alu_op(env, insn);
@@ -5768,13 +5767,13 @@ static int do_check(struct bpf_verifier_env *env)
5768	/* check that memory (src_reg + off) is readable,	5767	/* check that memory (src_reg + off) is readable,
5769	* the state of dst_reg will be updated by this func	5768	* the state of dst_reg will be updated by this func
5770	*/	5769	*/
5771	err = check_mem_access(env, insn_idx, insn->src_reg, insn->off,	5770	err = check_mem_access(env, env->insn_idx, insn->src_reg,
5772	BPF_SIZE(insn->code), BPF_READ,	5771	insn->off, BPF_SIZE(insn->code),
5773	insn->dst_reg, false);	5772	BPF_READ, insn->dst_reg, false);
5774	if (err)	5773	if (err)
5775	return err;	5774	return err;
5776		5775
5777	prev_src_type = &env->insn_aux_data[insn_idx].ptr_type;	5776	prev_src_type = &env->insn_aux_data[env->insn_idx].ptr_type;
5778		5777
5779	if (*prev_src_type == NOT_INIT) {	5778	if (*prev_src_type == NOT_INIT) {
5780	/* saw a valid insn	5779	/* saw a valid insn
@@ -5799,10 +5798,10 @@ static int do_check(struct bpf_verifier_env *env)
5799	enum bpf_reg_type *prev_dst_type, dst_reg_type;	5798	enum bpf_reg_type *prev_dst_type, dst_reg_type;
5800		5799
5801	if (BPF_MODE(insn->code) == BPF_XADD) {	5800	if (BPF_MODE(insn->code) == BPF_XADD) {
5802	err = check_xadd(env, insn_idx, insn);	5801	err = check_xadd(env, env->insn_idx, insn);
5803	if (err)	5802	if (err)
5804	return err;	5803	return err;
5805	insn_idx++;	5804	env->insn_idx++;
5806	continue;	5805	continue;
5807	}	5806	}
5808		5807
@@ -5818,13 +5817,13 @@ static int do_check(struct bpf_verifier_env *env)
5818	dst_reg_type = regs[insn->dst_reg].type;	5817	dst_reg_type = regs[insn->dst_reg].type;
5819		5818
5820	/* check that memory (dst_reg + off) is writeable */	5819	/* check that memory (dst_reg + off) is writeable */
5821	err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,	5820	err = check_mem_access(env, env->insn_idx, insn->dst_reg,
5822	BPF_SIZE(insn->code), BPF_WRITE,	5821	insn->off, BPF_SIZE(insn->code),
5823	insn->src_reg, false);	5822	BPF_WRITE, insn->src_reg, false);
5824	if (err)	5823	if (err)
5825	return err;	5824	return err;
5826		5825
5827	prev_dst_type = &env->insn_aux_data[insn_idx].ptr_type;	5826	prev_dst_type = &env->insn_aux_data[env->insn_idx].ptr_type;
5828		5827
5829	if (*prev_dst_type == NOT_INIT) {	5828	if (*prev_dst_type == NOT_INIT) {
5830	*prev_dst_type = dst_reg_type;	5829	*prev_dst_type = dst_reg_type;
@@ -5852,9 +5851,9 @@ static int do_check(struct bpf_verifier_env *env)
5852	}	5851	}
5853		5852
5854	/* check that memory (dst_reg + off) is writeable */	5853	/* check that memory (dst_reg + off) is writeable */
5855	err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off,	5854	err = check_mem_access(env, env->insn_idx, insn->dst_reg,
5856	BPF_SIZE(insn->code), BPF_WRITE,	5855	insn->off, BPF_SIZE(insn->code),
5857	-1, false);	5856	BPF_WRITE, -1, false);
5858	if (err)	5857	if (err)
5859	return err;	5858	return err;
5860		5859
@@ -5872,9 +5871,9 @@ static int do_check(struct bpf_verifier_env *env)
5872	}	5871	}
5873		5872
5874	if (insn->src_reg == BPF_PSEUDO_CALL)	5873	if (insn->src_reg == BPF_PSEUDO_CALL)
5875	err = check_func_call(env, insn, &insn_idx);	5874	err = check_func_call(env, insn, &env->insn_idx);
5876	else	5875	else
5877	err = check_helper_call(env, insn->imm, insn_idx);	5876	err = check_helper_call(env, insn->imm, env->insn_idx);
5878	if (err)	5877	if (err)
5879	return err;	5878	return err;
5880		5879
@@ -5887,7 +5886,7 @@ static int do_check(struct bpf_verifier_env *env)
5887	return -EINVAL;	5886	return -EINVAL;
5888	}	5887	}
5889		5888
5890	insn_idx += insn->off + 1;	5889	env->insn_idx += insn->off + 1;
5891	continue;	5890	continue;
5892		5891
5893	} else if (opcode == BPF_EXIT) {	5892	} else if (opcode == BPF_EXIT) {
@@ -5901,8 +5900,8 @@ static int do_check(struct bpf_verifier_env *env)
5901		5900
5902	if (state->curframe) {	5901	if (state->curframe) {
5903	/* exit from nested function */	5902	/* exit from nested function */
5904	prev_insn_idx = insn_idx;	5903	env->prev_insn_idx = env->insn_idx;
5905	err = prepare_func_exit(env, &insn_idx);	5904	err = prepare_func_exit(env, &env->insn_idx);
5906	if (err)	5905	if (err)
5907	return err;	5906	return err;
5908	do_print_state = true;	5907	do_print_state = true;
@@ -5932,7 +5931,8 @@ static int do_check(struct bpf_verifier_env *env)
5932	if (err)	5931	if (err)
5933	return err;	5932	return err;
5934	process_bpf_exit:	5933	process_bpf_exit:
5935	err = pop_stack(env, &prev_insn_idx, &insn_idx);	5934	err = pop_stack(env, &env->prev_insn_idx,
		5935	&env->insn_idx);
5936	if (err < 0) {	5936	if (err < 0) {
5937	if (err != -ENOENT)	5937	if (err != -ENOENT)
5938	return err;	5938	return err;
@@ -5942,7 +5942,7 @@ process_bpf_exit:
5942	continue;	5942	continue;
5943	}	5943	}
5944	} else {	5944	} else {
5945	err = check_cond_jmp_op(env, insn, &insn_idx);	5945	err = check_cond_jmp_op(env, insn, &env->insn_idx);
5946	if (err)	5946	if (err)
5947	return err;	5947	return err;
5948	}	5948	}
@@ -5959,8 +5959,8 @@ process_bpf_exit:
5959	if (err)	5959	if (err)
5960	return err;	5960	return err;
5961		5961
5962	insn_idx++;	5962	env->insn_idx++;
5963	env->insn_aux_data[insn_idx].seen = true;	5963	env->insn_aux_data[env->insn_idx].seen = true;
5964	} else {	5964	} else {
5965	verbose(env, "invalid BPF_LD mode\n");	5965	verbose(env, "invalid BPF_LD mode\n");
5966	return -EINVAL;	5966	return -EINVAL;
@@ -5970,7 +5970,7 @@ process_bpf_exit:
5970	return -EINVAL;	5970	return -EINVAL;
5971	}	5971	}
5972		5972
5973	insn_idx++;	5973	env->insn_idx++;
5974	}	5974	}
5975		5975
5976	verbose(env, "processed %d insns (limit %d), stack depth ",	5976	verbose(env, "processed %d insns (limit %d), stack depth ",