Commit a92a72a2 authored by Daniel Borkmann's avatar Daniel Borkmann
Browse files

Merge branch 'bpf-verifier-resilience' 



Alexei Starovoitov says:

====================
Three patches to improve verifier ability to handle pathological bpf
programs with a lot of branches:

- make sure prog_load syscall can be aborted
- improve branch taken analysis
- introduce per-insn complexity limit for unprivileged programs
====================

Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parents 025dceb0 ceefbc96

kernel/bpf/verifier.c

+89 −14
Original line number Diff line number Diff line
@@ -175,6 +175,7 @@ struct bpf_verifier_stack_elem { 


#define BPF_COMPLEXITY_LIMIT_INSNS	131072

#define BPF_COMPLEXITY_LIMIT_STACK	1024

#define BPF_COMPLEXITY_LIMIT_STATES	64



#define BPF_MAP_PTR_UNPRIV	1UL

#define BPF_MAP_PTR_POISON	((void *)((0xeB9FUL << 1) +	\
@@ -3751,6 +3752,79 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate, 
	}

}



/* compute branch direction of the expression "if (reg opcode val) goto target;"

 * and return:

 *  1 - branch will be taken and "goto target" will be executed

 *  0 - branch will not be taken and fall-through to next insn

 * -1 - unknown. Example: "if (reg < 5)" is unknown when register value range [0,10]

 */

static int is_branch_taken(struct bpf_reg_state *reg, u64 val, u8 opcode)

{

	if (__is_pointer_value(false, reg))

		return -1;



	switch (opcode) {

	case BPF_JEQ:

		if (tnum_is_const(reg->var_off))

			return !!tnum_equals_const(reg->var_off, val);

		break;

	case BPF_JNE:

		if (tnum_is_const(reg->var_off))

			return !tnum_equals_const(reg->var_off, val);

		break;

	case BPF_JGT:

		if (reg->umin_value > val)

			return 1;

		else if (reg->umax_value <= val)

			return 0;

		break;

	case BPF_JSGT:

		if (reg->smin_value > (s64)val)

			return 1;

		else if (reg->smax_value < (s64)val)

			return 0;

		break;

	case BPF_JLT:

		if (reg->umax_value < val)

			return 1;

		else if (reg->umin_value >= val)

			return 0;

		break;

	case BPF_JSLT:

		if (reg->smax_value < (s64)val)

			return 1;

		else if (reg->smin_value >= (s64)val)

			return 0;

		break;

	case BPF_JGE:

		if (reg->umin_value >= val)

			return 1;

		else if (reg->umax_value < val)

			return 0;

		break;

	case BPF_JSGE:

		if (reg->smin_value >= (s64)val)

			return 1;

		else if (reg->smax_value < (s64)val)

			return 0;

		break;

	case BPF_JLE:

		if (reg->umax_value <= val)

			return 1;

		else if (reg->umin_value > val)

			return 0;

		break;

	case BPF_JSLE:

		if (reg->smax_value <= (s64)val)

			return 1;

		else if (reg->smin_value > (s64)val)

			return 0;

		break;

	}



	return -1;

}



/* Adjusts the register min/max values in the case that the dst_reg is the

 * variable register that we are working on, and src_reg is a constant or we're

 * simply doing a BPF_K check.
@@ -4152,21 +4226,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, 


	dst_reg = &regs[insn->dst_reg];



	/* detect if R == 0 where R was initialized to zero earlier */

	if (BPF_SRC(insn->code) == BPF_K &&

	    (opcode == BPF_JEQ || opcode == BPF_JNE) &&

	    dst_reg->type == SCALAR_VALUE &&

	    tnum_is_const(dst_reg->var_off)) {

		if ((opcode == BPF_JEQ && dst_reg->var_off.value == insn->imm) ||

		    (opcode == BPF_JNE && dst_reg->var_off.value != insn->imm)) {

			/* if (imm == imm) goto pc+off;

			 * only follow the goto, ignore fall-through

			 */

	if (BPF_SRC(insn->code) == BPF_K) {

		int pred = is_branch_taken(dst_reg, insn->imm, opcode);



		if (pred == 1) {

			 /* only follow the goto, ignore fall-through */

			*insn_idx += insn->off;

			return 0;

		} else {

			/* if (imm != imm) goto pc+off;

			 * only follow fall-through branch, since

		} else if (pred == 0) {

			/* only follow fall-through branch, since

			 * that's where the program will go

			 */

			return 0;
@@ -4980,7 +5048,7 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) 
	struct bpf_verifier_state_list *new_sl;

	struct bpf_verifier_state_list *sl;

	struct bpf_verifier_state *cur = env->cur_state, *new;

	int i, j, err;

	int i, j, err, states_cnt = 0;



	sl = env->explored_states[insn_idx];

	if (!sl)
@@ -5007,8 +5075,12 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) 
			return 1;

		}

		sl = sl->next;

		states_cnt++;

	}



	if (!env->allow_ptr_leaks && states_cnt > BPF_COMPLEXITY_LIMIT_STATES)

		return 0;



	/* there were no equivalent states, remember current one.

	 * technically the current state is not proven to be safe yet,

	 * but it will either reach outer most bpf_exit (which means it's safe)
@@ -5148,6 +5220,9 @@ static int do_check(struct bpf_verifier_env *env) 
			goto process_bpf_exit;

		}



		if (signal_pending(current))

			return -EAGAIN;



		if (need_resched())

			cond_resched();

tools/testing/selftests/bpf/test_verifier.c

+2 −2
Original line number Diff line number Diff line
@@ -8576,7 +8576,7 @@ static struct bpf_test tests[] = { 
			BPF_JMP_IMM(BPF_JA, 0, 0, -7),

		},

		.fixup_map_hash_8b = { 4 },

		.errstr = "R0 invalid mem access 'inv'",

		.errstr = "unbounded min value",

		.result = REJECT,

	},

	{
@@ -10547,7 +10547,7 @@ static struct bpf_test tests[] = { 
		"check deducing bounds from const, 5",

		.insns = {

			BPF_MOV64_IMM(BPF_REG_0, 0),

			BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 0, 1),

			BPF_JMP_IMM(BPF_JSGE, BPF_REG_0, 1, 1),

			BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1),

			BPF_EXIT_INSN(),

		},

CRA Git | Maintained and supported by SUSTech CRA and CCSE