Commit 8ab4cdcf authored by Joanne Koong's avatar Joanne Koong Committed by Alexei Starovoitov
Browse files

bpf: Tidy up verifier check_func_arg() 



This patch does two things:

1. For matching against the arg type, the match should be against the
base type of the arg type, since the arg type can have different
bpf_type_flags set on it.

2. Uses switch casing to improve readability + efficiency.

Signed-off-by: default avatarJoanne Koong <joannelkoong@gmail.com>
Acked-by: default avatarHao Luo <haoluo@google.com>
Link: https://lore.kernel.org/r/20220712210603.123791-1-joannelkoong@gmail.com


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

kernel/bpf/verifier.c

+38 −28
Original line number Diff line number Diff line
@@ -5533,17 +5533,6 @@ static bool arg_type_is_mem_size(enum bpf_arg_type type) 
	       type == ARG_CONST_SIZE_OR_ZERO;

}



static bool arg_type_is_alloc_size(enum bpf_arg_type type)

{

	return type == ARG_CONST_ALLOC_SIZE_OR_ZERO;

}



static bool arg_type_is_int_ptr(enum bpf_arg_type type)

{

	return type == ARG_PTR_TO_INT ||

	       type == ARG_PTR_TO_LONG;

}



static bool arg_type_is_release(enum bpf_arg_type type)

{

	return type & OBJ_RELEASE;
@@ -5929,7 +5918,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
		meta->ref_obj_id = reg->ref_obj_id;

	}



	if (arg_type == ARG_CONST_MAP_PTR) {

	switch (base_type(arg_type)) {

	case ARG_CONST_MAP_PTR:

		/* bpf_map_xxx(map_ptr) call: remember that map_ptr */

		if (meta->map_ptr) {

			/* Use map_uid (which is unique id of inner map) to reject:
@@ -5954,7 +5944,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
		}

		meta->map_ptr = reg->map_ptr;

		meta->map_uid = reg->map_uid;

	} else if (arg_type == ARG_PTR_TO_MAP_KEY) {

		break;

	case ARG_PTR_TO_MAP_KEY:

		/* bpf_map_xxx(..., map_ptr, ..., key) call:

		 * check that [key, key + map->key_size) are within

		 * stack limits and initialized
@@ -5971,7 +5962,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
		err = check_helper_mem_access(env, regno,

					      meta->map_ptr->key_size, false,

					      NULL);

	} else if (base_type(arg_type) == ARG_PTR_TO_MAP_VALUE) {

		break;

	case ARG_PTR_TO_MAP_VALUE:

		if (type_may_be_null(arg_type) && register_is_null(reg))

			return 0;
@@ -5987,14 +5979,16 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
		err = check_helper_mem_access(env, regno,

					      meta->map_ptr->value_size, false,

					      meta);

	} else if (arg_type == ARG_PTR_TO_PERCPU_BTF_ID) {

		break;

	case ARG_PTR_TO_PERCPU_BTF_ID:

		if (!reg->btf_id) {

			verbose(env, "Helper has invalid btf_id in R%d\n", regno);

			return -EACCES;

		}

		meta->ret_btf = reg->btf;

		meta->ret_btf_id = reg->btf_id;

	} else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {

		break;

	case ARG_PTR_TO_SPIN_LOCK:

		if (meta->func_id == BPF_FUNC_spin_lock) {

			if (process_spin_lock(env, regno, true))

				return -EACCES;
@@ -6005,12 +5999,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
			verbose(env, "verifier internal error\n");

			return -EFAULT;

		}

	} else if (arg_type == ARG_PTR_TO_TIMER) {

		break;

	case ARG_PTR_TO_TIMER:

		if (process_timer_func(env, regno, meta))

			return -EACCES;

	} else if (arg_type == ARG_PTR_TO_FUNC) {

		break;

	case ARG_PTR_TO_FUNC:

		meta->subprogno = reg->subprogno;

	} else if (base_type(arg_type) == ARG_PTR_TO_MEM) {

		break;

	case ARG_PTR_TO_MEM:

		/* The access to this pointer is only checked when we hit the

		 * next is_mem_size argument below.

		 */
@@ -6020,11 +6017,14 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
						      fn->arg_size[arg], false,

						      meta);

		}

	} else if (arg_type_is_mem_size(arg_type)) {

		bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);



		err = check_mem_size_reg(env, reg, regno, zero_size_allowed, meta);

	} else if (arg_type_is_dynptr(arg_type)) {

		break;

	case ARG_CONST_SIZE:

		err = check_mem_size_reg(env, reg, regno, false, meta);

		break;

	case ARG_CONST_SIZE_OR_ZERO:

		err = check_mem_size_reg(env, reg, regno, true, meta);

		break;

	case ARG_PTR_TO_DYNPTR:

		if (arg_type & MEM_UNINIT) {

			if (!is_dynptr_reg_valid_uninit(env, reg)) {

				verbose(env, "Dynptr has to be an uninitialized dynptr\n");
@@ -6058,21 +6058,28 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
				err_extra, arg + 1);

			return -EINVAL;

		}

	} else if (arg_type_is_alloc_size(arg_type)) {

		break;

	case ARG_CONST_ALLOC_SIZE_OR_ZERO:

		if (!tnum_is_const(reg->var_off)) {

			verbose(env, "R%d is not a known constant'\n",

				regno);

			return -EACCES;

		}

		meta->mem_size = reg->var_off.value;

	} else if (arg_type_is_int_ptr(arg_type)) {

		break;

	case ARG_PTR_TO_INT:

	case ARG_PTR_TO_LONG:

	{

		int size = int_ptr_type_to_size(arg_type);



		err = check_helper_mem_access(env, regno, size, false, meta);

		if (err)

			return err;

		err = check_ptr_alignment(env, reg, 0, size, true);

	} else if (arg_type == ARG_PTR_TO_CONST_STR) {

		break;

	}

	case ARG_PTR_TO_CONST_STR:

	{

		struct bpf_map *map = reg->map_ptr;

		int map_off;

		u64 map_addr;
@@ -6111,9 +6118,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg, 
			verbose(env, "string is not zero-terminated\n");

			return -EINVAL;

		}

	} else if (arg_type == ARG_PTR_TO_KPTR) {

		break;

	}

	case ARG_PTR_TO_KPTR:

		if (process_kptr_func(env, regno, meta))

			return -EACCES;

		break;

	}



	return err;