Skip to content

Commit

Permalink
bpf: Tidy up verifier check_func_arg()
Browse files Browse the repository at this point in the history 
[ Upstream commit 8ab4cdc ]

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: Joanne Koong <joannelkoong@gmail.com>
Acked-by: Hao Luo <haoluo@google.com>
Link: https://lore.kernel.org/r/20220712210603.123791-1-joannelkoong@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
@joannekoong @gregkh
joannekoong authored and gregkh committed Sep 8, 2022
1 parent 840fbb6 commit 1060864
Showing 1 changed file with 38 additions and 28 deletions.
66 changes: 38 additions & 28 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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.
*/
Expand All @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 1060864

Please sign in to comment.