Commit 329a6720 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf-verifier-range-computation-improvements' 

Cupertino Miranda says:

====================
bpf/verifier: range computation improvements

Hi everyone,

This is what I hope to be the last version. :)

Regards,
Cupertino

Changes from v1:
 - Reordered patches in the series.
 - Fix refactor to be acurate with original code.
 - Fixed other mentioned small problems.

Changes from v2:
 - Added a patch to replace mark_reg_unknowon for __mark_reg_unknown in
   the context of range computation.
 - Reverted implementation of refactor to v1 which used a simpler
   boolean return value in check function.
 - Further relaxed MUL to allow it to still compute a range when neither
   of its registers is a known value.
 - Simplified tests based on Eduards example.
 - Added messages in selftest commits.

Changes from v3:
 - Improved commit message of patch nr 1.
 - Coding style fixes.
 - Improve XOR and OR tests.
 - Made function calls to pass struct bpf_reg_state pointer instead.
 - Improved final code as a last patch.

Changes from v4:
 - Merged patch nr 7 in 2.

====================

Link: https://lore.kernel.org/r/20240506141849.185293-1-cupertino.miranda@oracle.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 41b307ad 92956786

kernel/bpf/verifier.c

+41 −65
Original line number Diff line number Diff line
@@ -13878,6 +13878,46 @@ static void scalar_min_max_arsh(struct bpf_reg_state *dst_reg, 
	__update_reg_bounds(dst_reg);

}









static bool is_safe_to_compute_dst_reg_range(struct bpf_insn *insn,









					     const struct bpf_reg_state *src_reg)









{









	bool src_is_const = false;









	u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;

















	if (insn_bitness == 32) {









		if (tnum_subreg_is_const(src_reg->var_off)









		    && src_reg->s32_min_value == src_reg->s32_max_value









		    && src_reg->u32_min_value == src_reg->u32_max_value)









			src_is_const = true;









	} else {









		if (tnum_is_const(src_reg->var_off)









		    && src_reg->smin_value == src_reg->smax_value









		    && src_reg->umin_value == src_reg->umax_value)









			src_is_const = true;









	}

















	switch (BPF_OP(insn->code)) {









	case BPF_ADD:









	case BPF_SUB:









	case BPF_AND:









	case BPF_XOR:









	case BPF_OR:









	case BPF_MUL:









		return true;

















	/* Shift operators range is only computable if shift dimension operand









	 * is a constant. Shifts greater than 31 or 63 are undefined. This









	 * includes shifts by a negative number.









	 */









	case BPF_LSH:









	case BPF_RSH:









	case BPF_ARSH:









		return (src_is_const && src_reg->umax_value < insn_bitness);









	default:









		return false;









	}









}

















/* WARNING: This function does calculations on 64-bit values, but the actual









 * execution may occur on 32-bit values. Therefore, things like bitshifts









 * need extra checks in the 32-bit case.








@@ -13887,53 +13927,11 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,







				      struct bpf_reg_state *dst_reg,









				      struct bpf_reg_state src_reg)









{









	struct bpf_reg_state *regs = cur_regs(env);









	u8 opcode = BPF_OP(insn->code);









	bool src_known;









	s64 smin_val, smax_val;









	u64 umin_val, umax_val;









	s32 s32_min_val, s32_max_val;









	u32 u32_min_val, u32_max_val;









	u64 insn_bitness = (BPF_CLASS(insn->code) == BPF_ALU64) ? 64 : 32;









	bool alu32 = (BPF_CLASS(insn->code) != BPF_ALU64);









	int ret;

















	smin_val = src_reg.smin_value;









	smax_val = src_reg.smax_value;









	umin_val = src_reg.umin_value;









	umax_val = src_reg.umax_value;

















	s32_min_val = src_reg.s32_min_value;









	s32_max_val = src_reg.s32_max_value;









	u32_min_val = src_reg.u32_min_value;









	u32_max_val = src_reg.u32_max_value;

















	if (alu32) {









		src_known = tnum_subreg_is_const(src_reg.var_off);









		if ((src_known &&









		     (s32_min_val != s32_max_val || u32_min_val != u32_max_val)) ||









		    s32_min_val > s32_max_val || u32_min_val > u32_max_val) {









			/* Taint dst register if offset had invalid bounds









			 * derived from e.g. dead branches.









			 */









			__mark_reg_unknown(env, dst_reg);









			return 0;









		}









	} else {









		src_known = tnum_is_const(src_reg.var_off);









		if ((src_known &&









		     (smin_val != smax_val || umin_val != umax_val)) ||









		    smin_val > smax_val || umin_val > umax_val) {









			/* Taint dst register if offset had invalid bounds









			 * derived from e.g. dead branches.









			 */









			__mark_reg_unknown(env, dst_reg);









			return 0;









		}









	}

















	if (!src_known &&









	    opcode != BPF_ADD && opcode != BPF_SUB && opcode != BPF_AND) {









	if (!is_safe_to_compute_dst_reg_range(insn, &src_reg)) {









		__mark_reg_unknown(env, dst_reg);









		return 0;









	}








@@ -13990,46 +13988,24 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,







		scalar_min_max_xor(dst_reg, &src_reg);









		break;









	case BPF_LSH:









		if (umax_val >= insn_bitness) {









			/* Shifts greater than 31 or 63 are undefined.









			 * This includes shifts by a negative number.









			 */









			mark_reg_unknown(env, regs, insn->dst_reg);









			break;









		}









		if (alu32)









			scalar32_min_max_lsh(dst_reg, &src_reg);









		else









			scalar_min_max_lsh(dst_reg, &src_reg);









		break;









	case BPF_RSH:









		if (umax_val >= insn_bitness) {









			/* Shifts greater than 31 or 63 are undefined.









			 * This includes shifts by a negative number.









			 */









			mark_reg_unknown(env, regs, insn->dst_reg);









			break;









		}









		if (alu32)









			scalar32_min_max_rsh(dst_reg, &src_reg);









		else









			scalar_min_max_rsh(dst_reg, &src_reg);









		break;









	case BPF_ARSH:









		if (umax_val >= insn_bitness) {









			/* Shifts greater than 31 or 63 are undefined.









			 * This includes shifts by a negative number.









			 */









			mark_reg_unknown(env, regs, insn->dst_reg);









			break;









		}









		if (alu32)









			scalar32_min_max_arsh(dst_reg, &src_reg);









		else









			scalar_min_max_arsh(dst_reg, &src_reg);









		break;









	default:









		mark_reg_unknown(env, regs, insn->dst_reg);









		break;









	}








































tools/testing/selftests/bpf/progs/verifier_bounds.c



+63
−0






























Original line number
Diff line number
Diff line







@@ -885,6 +885,69 @@ l1_%=:	r0 = 0;						\







	: __clobber_all);









}



















SEC("socket")









__description("bounds check for non const xor src dst")









__success __log_level(2)









__msg("5: (af) r0 ^= r6                      ; R0_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=431,var_off=(0x0; 0x1af))")









__naked void non_const_xor_src_dst(void)









{









	asm volatile ("					\









	call %[bpf_get_prandom_u32];                    \









	r6 = r0;					\









	call %[bpf_get_prandom_u32];                    \









	r6 &= 0xaf;					\









	r0 &= 0x1a0;					\









	r0 ^= r6;					\









	exit;						\









"	:









	: __imm(bpf_map_lookup_elem),









	__imm_addr(map_hash_8b),









	__imm(bpf_get_prandom_u32)









	: __clobber_all);









}



















SEC("socket")









__description("bounds check for non const or src dst")









__success __log_level(2)









__msg("5: (4f) r0 |= r6                      ; R0_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=431,var_off=(0x0; 0x1af))")









__naked void non_const_or_src_dst(void)









{









	asm volatile ("					\









	call %[bpf_get_prandom_u32];                    \









	r6 = r0;					\









	call %[bpf_get_prandom_u32];                    \









	r6 &= 0xaf;					\









	r0 &= 0x1a0;					\









	r0 |= r6;					\









	exit;						\









"	:









	: __imm(bpf_map_lookup_elem),









	__imm_addr(map_hash_8b),









	__imm(bpf_get_prandom_u32)









	: __clobber_all);









}



















SEC("socket")









__description("bounds check for non const mul regs")









__success __log_level(2)









__msg("5: (2f) r0 *= r6                      ; R0_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=3825,var_off=(0x0; 0xfff))")









__naked void non_const_mul_regs(void)









{









	asm volatile ("					\









	call %[bpf_get_prandom_u32];                    \









	r6 = r0;					\









	call %[bpf_get_prandom_u32];                    \









	r6 &= 0xff;					\









	r0 &= 0x0f;					\









	r0 *= r6;					\









	exit;						\









"	:









	: __imm(bpf_map_lookup_elem),









	__imm_addr(map_hash_8b),









	__imm(bpf_get_prandom_u32)









	: __clobber_all);









}



















SEC("socket")









__description("bounds checks after 32-bit truncation. test 1")









__success __failure_unpriv __msg_unpriv("R0 leaks addr")