From: Daniel Borkmann <daniel@iogearbox.net>

Date: Thu, 3 Jan 2019 00:58:27 +0100

Subject: bpf: move {prev_,}insn_idx into verifier env

Patch-mainline: v5.0-rc1

Git-commit: c08435ec7f2bc8f4109401f696fd55159b4b40cb

References: bsc#1068032 CVE-2017-5753 bsc#1124055 CVE-2019-7308

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>

Acked-by: Gary Lin <glin@suse.com>

---

 include/linux/bpf_verifier.h |    2 +

 kernel/bpf/verifier.c        |   65 +++++++++++++++++++++----------------------

 2 files changed, 34 insertions(+), 33 deletions(-)

--- a/include/linux/bpf_verifier.h

+++ b/include/linux/bpf_verifier.h

@@ -149,6 +149,8 @@ struct bpf_ext_analyzer_ops {

  * 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 */

--- a/kernel/bpf/verifier.c

+++ b/kernel/bpf/verifier.c

@@ -3751,7 +3751,6 @@ static int do_check(struct bpf_verifier_

 	struct bpf_insn *insns = env->prog->insnsi;

 	struct bpf_reg_state *regs;

 	int insn_cnt = env->prog->len;

-	int insn_idx, prev_insn_idx = 0;

 	int insn_processed = 0;

 	bool do_print_state = false;

@@ -3761,19 +3760,18 @@ static int do_check(struct bpf_verifier_

 	env->cur_state = state;

 	init_reg_state(env, state->regs);

 	state->parent = NULL;

-	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) {

@@ -3783,7 +3781,7 @@ static int do_check(struct bpf_verifier_

 			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) {

@@ -3791,9 +3789,9 @@ static int do_check(struct bpf_verifier_

 			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;

 		}

@@ -3803,26 +3801,26 @@ static int do_check(struct bpf_verifier_

 		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);

 			do_print_state = false;

 		}

 		if (env->log.level) {

-			verbose(env, "%d: ", insn_idx);

+			verbose(env, "%d: ", env->insn_idx);

 			print_bpf_insn(verbose, env, insn,

 				       env->allow_ptr_leaks);

 		}

-		err = ext_analyzer_insn_hook(env, insn_idx, prev_insn_idx);

+		err = ext_analyzer_insn_hook(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);

 			if (err)

@@ -3847,13 +3845,13 @@ static int do_check(struct bpf_verifier_

 			/* 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

@@ -3880,10 +3878,10 @@ static int do_check(struct bpf_verifier_

 			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;

 			}

@@ -3899,13 +3897,13 @@ static int do_check(struct bpf_verifier_

 			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;

@@ -3934,9 +3932,9 @@ static int do_check(struct bpf_verifier_

 			}

 			/* 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;

@@ -3952,7 +3950,7 @@ static int do_check(struct bpf_verifier_

 					return -EINVAL;

 				}

-				err = check_call(env, insn->imm, insn_idx);

+				err = check_call(env, insn->imm, env->insn_idx);

 				if (err)

 					return err;

@@ -3965,7 +3963,7 @@ static int do_check(struct bpf_verifier_

 					return -EINVAL;

 				}

-				insn_idx += insn->off + 1;

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

 				continue;

 			} else if (opcode == BPF_EXIT) {

@@ -3993,7 +3991,8 @@ static int do_check(struct bpf_verifier_

 				}

 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;

@@ -4003,7 +4002,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;

 			}

@@ -4020,8 +4019,8 @@ process_bpf_exit:

 				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;

@@ -4031,7 +4030,7 @@ process_bpf_exit:

 			return -EINVAL;

 		}

-		insn_idx++;

+		env->insn_idx++;

 	}

 	verbose(env, "processed %d insns, stack depth %d\n", insn_processed,