Commit a640de4c authored by Andrii Nakryiko's avatar Andrii Nakryiko
Browse files

Merge branch 'bpf-simplify-checking-size-of-helper-accesses' 

Andrei Matei says:

====================
bpf: Simplify checking size of helper accesses

v3->v4:
- kept only the minimal change, undoing debatable changes (Andrii)
- dropped the second patch from before, with changes to the error
  message (Andrii)
- extracted the new test into a separate patch (Andrii)
- added Acked by Andrii

v2->v3:
- split the error-logging function to a separate patch (Andrii)
- make the error buffers smaller (Andrii)
- include size of memory region for PTR_TO_MEM (Andrii)
- nits from Andrii and Eduard

v1->v2:
- make the error message include more info about the context of the
  zero-sized access (Andrii)
====================

Link: https://lore.kernel.org/r/20231221232225.568730-1-andreimatei1@gmail.com


Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents 2ab1efad 72187506

kernel/bpf/verifier.c

+4 −6
Original line number Diff line number Diff line
@@ -7279,12 +7279,10 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, 
		return -EACCES;

	}









	if (reg->umin_value == 0) {









		err = check_helper_mem_access(env, regno - 1, 0,









					      zero_size_allowed,









					      meta);









		if (err)









			return err;









	if (reg->umin_value == 0 && !zero_size_allowed) {









		verbose(env, "R%d invalid zero-sized read: u64=[%lld,%lld]\n",









			regno, reg->umin_value, reg->umax_value);









		return -EACCES;









	}

















	if (reg->umax_value >= BPF_MAX_VAR_SIZ) {
































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



+41
−4






























Original line number
Diff line number
Diff line







@@ -89,9 +89,14 @@ l0_%=:	exit;						\







	: __clobber_all);









}



















/* Call a function taking a pointer and a size which doesn't allow the size to









 * be zero (i.e. bpf_trace_printk() declares the second argument to be









 * ARG_CONST_SIZE, not ARG_CONST_SIZE_OR_ZERO). We attempt to pass zero for the









 * size and expect to fail.









 */









SEC("tracepoint")









__description("helper access to map: empty range")









__failure __msg("invalid access to map value, value_size=48 off=0 size=0")









__failure __msg("R2 invalid zero-sized read: u64=[0,0]")









__naked void access_to_map_empty_range(void)









{









	asm volatile ("					\








@@ -113,6 +118,38 @@ l0_%=:	exit;						\







	: __clobber_all);









}



















/* Like the test above, but this time the size register is not known to be zero;









 * its lower-bound is zero though, which is still unacceptable.









 */









SEC("tracepoint")









__description("helper access to map: possibly-empty ange")









__failure __msg("R2 invalid zero-sized read: u64=[0,4]")









__naked void access_to_map_possibly_empty_range(void)









{









	asm volatile ("                                         \









	r2 = r10;                                               \









	r2 += -8;                                               \









	r1 = 0;                                                 \









	*(u64*)(r2 + 0) = r1;                                   \









	r1 = %[map_hash_48b] ll;                                \









	call %[bpf_map_lookup_elem];                            \









	if r0 == 0 goto l0_%=;                                  \









	r1 = r0;                                                \









	/* Read an unknown value */                             \









	r7 = *(u64*)(r0 + 0);                                   \









	/* Make it small and positive, to avoid other errors */ \









	r7 &= 4;                                                \









	r2 = 0;                                                 \









	r2 += r7;                                               \









	call %[bpf_trace_printk];                               \









l0_%=:	exit;                                               \









"	:









	: __imm(bpf_map_lookup_elem),









	  __imm(bpf_trace_printk),









	  __imm_addr(map_hash_48b)









	: __clobber_all);









}



















SEC("tracepoint")









__description("helper access to map: out-of-bound range")









__failure __msg("invalid access to map value, value_size=48 off=0 size=56")








@@ -221,7 +258,7 @@ l0_%=:	exit;						\

















SEC("tracepoint")









__description("helper access to adjusted map (via const imm): empty range")









__failure __msg("invalid access to map value, value_size=48 off=4 size=0")









__failure __msg("R2 invalid zero-sized read")









__naked void via_const_imm_empty_range(void)









{









	asm volatile ("					\








@@ -386,7 +423,7 @@ l0_%=:	exit;						\

















SEC("tracepoint")









__description("helper access to adjusted map (via const reg): empty range")









__failure __msg("R1 min value is outside of the allowed memory range")









__failure __msg("R2 invalid zero-sized read")









__naked void via_const_reg_empty_range(void)









{









	asm volatile ("					\








@@ -556,7 +593,7 @@ l0_%=:	exit;						\

















SEC("tracepoint")









__description("helper access to adjusted map (via variable): empty range")









__failure __msg("R1 min value is outside of the allowed memory range")









__failure __msg("R2 invalid zero-sized read")









__naked void map_via_variable_empty_range(void)









{









	asm volatile ("					\
































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



+1
−1






























Original line number
Diff line number
Diff line







@@ -64,7 +64,7 @@ __naked void load_bytes_negative_len_2(void)

















SEC("tc")









__description("raw_stack: skb_load_bytes, zero len")









__failure __msg("invalid zero-sized read")









__failure __msg("R4 invalid zero-sized read: u64=[0,0]")









__naked void skb_load_bytes_zero_len(void)









{









	asm volatile ("					\