mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	bpf: improve verifier branch analysis
pathological bpf programs may try to force verifier to explode in
the number of branch states:
  20: (d5) if r1 s<= 0x24000028 goto pc+0
  21: (b5) if r0 <= 0xe1fa20 goto pc+2
  22: (d5) if r1 s<= 0x7e goto pc+0
  23: (b5) if r0 <= 0xe880e000 goto pc+0
  24: (c5) if r0 s< 0x2100ecf4 goto pc+0
  25: (d5) if r1 s<= 0xe880e000 goto pc+1
  26: (c5) if r0 s< 0xf4041810 goto pc+0
  27: (d5) if r1 s<= 0x1e007e goto pc+0
  28: (b5) if r0 <= 0xe86be000 goto pc+0
  29: (07) r0 += 16614
  30: (c5) if r0 s< 0x6d0020da goto pc+0
  31: (35) if r0 >= 0x2100ecf4 goto pc+0
Teach verifier to recognize always taken and always not taken branches.
This analysis is already done for == and != comparison.
Expand it to all other branches.
It also helps real bpf programs to be verified faster:
                       before  after
bpf_lb-DLB_L3.o         2003    1940
bpf_lb-DLB_L4.o         3173    3089
bpf_lb-DUNKNOWN.o       1080    1065
bpf_lxc-DDROP_ALL.o     29584   28052
bpf_lxc-DUNKNOWN.o      36916   35487
bpf_netdev.o            11188   10864
bpf_overlay.o           6679    6643
bpf_lcx_jit.o           39555   38437
Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Edward Cree <ecree@solarflare.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
			
			
This commit is contained in:
		
							parent
							
								
									c3494801cd
								
							
						
					
					
						commit
						4f7b3e8258
					
				
					 2 changed files with 82 additions and 15 deletions
				
			
		| 
						 | 
					@ -3751,6 +3751,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
 | 
					/* 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
 | 
					 * variable register that we are working on, and src_reg is a constant or we're
 | 
				
			||||||
 * simply doing a BPF_K check.
 | 
					 * simply doing a BPF_K check.
 | 
				
			||||||
| 
						 | 
					@ -4152,21 +4225,15 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dst_reg = ®s[insn->dst_reg];
 | 
						dst_reg = ®s[insn->dst_reg];
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	/* detect if R == 0 where R was initialized to zero earlier */
 | 
						if (BPF_SRC(insn->code) == BPF_K) {
 | 
				
			||||||
	if (BPF_SRC(insn->code) == BPF_K &&
 | 
							int pred = is_branch_taken(dst_reg, insn->imm, opcode);
 | 
				
			||||||
	    (opcode == BPF_JEQ || opcode == BPF_JNE) &&
 | 
					
 | 
				
			||||||
	    dst_reg->type == SCALAR_VALUE &&
 | 
							if (pred == 1) {
 | 
				
			||||||
	    tnum_is_const(dst_reg->var_off)) {
 | 
								 /* only follow the goto, ignore fall-through */
 | 
				
			||||||
		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
 | 
					 | 
				
			||||||
			 */
 | 
					 | 
				
			||||||
			*insn_idx += insn->off;
 | 
								*insn_idx += insn->off;
 | 
				
			||||||
			return 0;
 | 
								return 0;
 | 
				
			||||||
		} else {
 | 
							} else if (pred == 0) {
 | 
				
			||||||
			/* if (imm != imm) goto pc+off;
 | 
								/* only follow fall-through branch, since
 | 
				
			||||||
			 * only follow fall-through branch, since
 | 
					 | 
				
			||||||
			 * that's where the program will go
 | 
								 * that's where the program will go
 | 
				
			||||||
			 */
 | 
								 */
 | 
				
			||||||
			return 0;
 | 
								return 0;
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -8576,7 +8576,7 @@ static struct bpf_test tests[] = {
 | 
				
			||||||
			BPF_JMP_IMM(BPF_JA, 0, 0, -7),
 | 
								BPF_JMP_IMM(BPF_JA, 0, 0, -7),
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
		.fixup_map_hash_8b = { 4 },
 | 
							.fixup_map_hash_8b = { 4 },
 | 
				
			||||||
		.errstr = "R0 invalid mem access 'inv'",
 | 
							.errstr = "unbounded min value",
 | 
				
			||||||
		.result = REJECT,
 | 
							.result = REJECT,
 | 
				
			||||||
	},
 | 
						},
 | 
				
			||||||
	{
 | 
						{
 | 
				
			||||||
| 
						 | 
					@ -10547,7 +10547,7 @@ static struct bpf_test tests[] = {
 | 
				
			||||||
		"check deducing bounds from const, 5",
 | 
							"check deducing bounds from const, 5",
 | 
				
			||||||
		.insns = {
 | 
							.insns = {
 | 
				
			||||||
			BPF_MOV64_IMM(BPF_REG_0, 0),
 | 
								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_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1),
 | 
				
			||||||
			BPF_EXIT_INSN(),
 | 
								BPF_EXIT_INSN(),
 | 
				
			||||||
		},
 | 
							},
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue