Commit 7e0dac28 authored by Joanne Koong's avatar Joanne Koong Committed by Alexei Starovoitov
Browse files

bpf: Refactor process_dynptr_func 



This change cleans up process_dynptr_func's flow to be more intuitive
and updates some comments with more context.

Signed-off-by: default avatarJoanne Koong <joannelkoong@gmail.com>
Link: https://lore.kernel.org/r/20230301154953.641654-3-joannelkoong@gmail.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 2f464393

include/linux/bpf_verifier.h

+0 −3
Original line number Diff line number Diff line
@@ -616,9 +616,6 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env, 
			   enum bpf_arg_type arg_type);

int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,

		   u32 regno, u32 mem_size);

struct bpf_call_arg_meta;

int process_dynptr_func(struct bpf_verifier_env *env, int regno,

			enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta);



/* this lives here instead of in bpf.h because it needs to dereference tgt_prog */

static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,

kernel/bpf/verifier.c

+31 −31
Original line number Diff line number Diff line
@@ -959,39 +959,49 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env, 
	return 0;

}









static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg,









				       int spi)









static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg)









{









	int spi;

















	if (reg->type == CONST_PTR_TO_DYNPTR)









		return false;

















	/* For -ERANGE (i.e. spi not falling into allocated stack slots), we









	 * will do check_mem_access to check and update stack bounds later, so









	 * return true for that case.









	spi = dynptr_get_spi(env, reg);

















	/* -ERANGE (i.e. spi not falling into allocated stack slots) isn't an









	 * error because this just means the stack state hasn't been updated yet.









	 * We will do check_mem_access to check and update stack bounds later.









	 */









	if (spi < 0)









		return spi == -ERANGE;









	/* We allow overwriting existing unreferenced STACK_DYNPTR slots, see









	 * mark_stack_slots_dynptr which calls destroy_if_dynptr_stack_slot to









	 * ensure dynptr objects at the slots we are touching are completely









	 * destructed before we reinitialize them for a new one. For referenced









	 * ones, destroy_if_dynptr_stack_slot returns an error early instead of









	 * delaying it until the end where the user will get "Unreleased









	if (spi < 0 && spi != -ERANGE)









		return false;

















	/* We don't need to check if the stack slots are marked by previous









	 * dynptr initializations because we allow overwriting existing unreferenced









	 * STACK_DYNPTR slots, see mark_stack_slots_dynptr which calls









	 * destroy_if_dynptr_stack_slot to ensure dynptr objects at the slots we are









	 * touching are completely destructed before we reinitialize them for a new









	 * one. For referenced ones, destroy_if_dynptr_stack_slot returns an error early









	 * instead of delaying it until the end where the user will get "Unreleased









	 * reference" error.









	 */









	return true;









}

















static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,









				     int spi)









static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg)









{









	struct bpf_func_state *state = func(env, reg);









	int i;









	int i, spi;

















	/* This already represents first slot of initialized bpf_dynptr */









	/* This already represents first slot of initialized bpf_dynptr.









	 *









	 * CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to









	 * check_func_arg_reg_off's logic, so we don't need to check its









	 * offset and alignment.









	 */









	if (reg->type == CONST_PTR_TO_DYNPTR)









		return true;

















	spi = dynptr_get_spi(env, reg);









	if (spi < 0)









		return false;









	if (!state->stack[spi].spilled_ptr.dynptr.first_slot)








@@ -6215,11 +6225,10 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,







 * Helpers which do not mutate the bpf_dynptr set MEM_RDONLY in their argument









 * type, and declare it as 'const struct bpf_dynptr *' in their prototype.









 */









int process_dynptr_func(struct bpf_verifier_env *env, int regno,









static int process_dynptr_func(struct bpf_verifier_env *env, int regno,









			       enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta)









{









	struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];









	int spi = 0;

















	/* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to an









	 * ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*):








@@ -6228,15 +6237,6 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno,







		verbose(env, "verifier internal error: misconfigured dynptr helper type flags\n");









		return -EFAULT;









	}









	/* CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to









	 * check_func_arg_reg_off's logic. We only need to check offset









	 * and its alignment for PTR_TO_STACK.









	 */









	if (reg->type == PTR_TO_STACK) {









		spi = dynptr_get_spi(env, reg);









		if (spi < 0 && spi != -ERANGE)









			return spi;









	}

















	/*  MEM_UNINIT - Points to memory that is an appropriate candidate for









	 *		 constructing a mutable bpf_dynptr object.








@@ -6254,7 +6254,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno,







	 *		 to.









	 */









	if (arg_type & MEM_UNINIT) {









		if (!is_dynptr_reg_valid_uninit(env, reg, spi)) {









		if (!is_dynptr_reg_valid_uninit(env, reg)) {









			verbose(env, "Dynptr has to be an uninitialized dynptr\n");









			return -EINVAL;









		}








@@ -6277,7 +6277,7 @@ int process_dynptr_func(struct bpf_verifier_env *env, int regno,







			return -EINVAL;









		}

















		if (!is_dynptr_reg_valid_init(env, reg, spi)) {









		if (!is_dynptr_reg_valid_init(env, reg)) {









			verbose(env,









				"Expected an initialized dynptr as arg #%d\n",









				regno);