Commit 90133415 authored by Daniel Borkmann's avatar Daniel Borkmann Committed by Alexei Starovoitov

bpf, verifier: detect misconfigured mem, size argument pair

I've seen two patch proposals now for helper additions that used
ARG_PTR_TO_MEM or similar in reg_X but no corresponding ARG_CONST_SIZE
in reg_X+1. Verifier won't complain in such case, but it will omit
verifying the memory passed to the helper thus ending up badly.
Detect such buggy helper function signature and bail out during
verification rather than finding them through review.
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 417f1d9f
...@@ -1837,6 +1837,19 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno, ...@@ -1837,6 +1837,19 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
} }
} }
static bool arg_type_is_mem_ptr(enum bpf_arg_type type)
{
return type == ARG_PTR_TO_MEM ||
type == ARG_PTR_TO_MEM_OR_NULL ||
type == ARG_PTR_TO_UNINIT_MEM;
}
static bool arg_type_is_mem_size(enum bpf_arg_type type)
{
return type == ARG_CONST_SIZE ||
type == ARG_CONST_SIZE_OR_ZERO;
}
static int check_func_arg(struct bpf_verifier_env *env, u32 regno, static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
enum bpf_arg_type arg_type, enum bpf_arg_type arg_type,
struct bpf_call_arg_meta *meta) struct bpf_call_arg_meta *meta)
...@@ -1886,9 +1899,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno, ...@@ -1886,9 +1899,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
expected_type = PTR_TO_CTX; expected_type = PTR_TO_CTX;
if (type != expected_type) if (type != expected_type)
goto err_type; goto err_type;
} else if (arg_type == ARG_PTR_TO_MEM || } else if (arg_type_is_mem_ptr(arg_type)) {
arg_type == ARG_PTR_TO_MEM_OR_NULL ||
arg_type == ARG_PTR_TO_UNINIT_MEM) {
expected_type = PTR_TO_STACK; expected_type = PTR_TO_STACK;
/* One exception here. In case function allows for NULL to be /* One exception here. In case function allows for NULL to be
* passed in as argument, it's a SCALAR_VALUE type. Final test * passed in as argument, it's a SCALAR_VALUE type. Final test
...@@ -1949,25 +1960,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno, ...@@ -1949,25 +1960,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
err = check_stack_boundary(env, regno, err = check_stack_boundary(env, regno,
meta->map_ptr->value_size, meta->map_ptr->value_size,
false, NULL); false, NULL);
} else if (arg_type == ARG_CONST_SIZE || } else if (arg_type_is_mem_size(arg_type)) {
arg_type == ARG_CONST_SIZE_OR_ZERO) {
bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO); bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
/* bpf_xxx(..., buf, len) call will access 'len' bytes
* from stack pointer 'buf'. Check it
* note: regno == len, regno - 1 == buf
*/
if (regno == 0) {
/* kernel subsystem misconfigured verifier */
verbose(env,
"ARG_CONST_SIZE cannot be first argument\n");
return -EACCES;
}
/* The register is SCALAR_VALUE; the access check /* The register is SCALAR_VALUE; the access check
* happens using its boundaries. * happens using its boundaries.
*/ */
if (!tnum_is_const(reg->var_off)) if (!tnum_is_const(reg->var_off))
/* For unprivileged variable accesses, disable raw /* For unprivileged variable accesses, disable raw
* mode so that the program is required to * mode so that the program is required to
...@@ -2111,7 +2109,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env, ...@@ -2111,7 +2109,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
return -EINVAL; return -EINVAL;
} }
static int check_raw_mode(const struct bpf_func_proto *fn) static bool check_raw_mode_ok(const struct bpf_func_proto *fn)
{ {
int count = 0; int count = 0;
...@@ -2126,7 +2124,44 @@ static int check_raw_mode(const struct bpf_func_proto *fn) ...@@ -2126,7 +2124,44 @@ static int check_raw_mode(const struct bpf_func_proto *fn)
if (fn->arg5_type == ARG_PTR_TO_UNINIT_MEM) if (fn->arg5_type == ARG_PTR_TO_UNINIT_MEM)
count++; count++;
return count > 1 ? -EINVAL : 0; /* We only support one arg being in raw mode at the moment,
* which is sufficient for the helper functions we have
* right now.
*/
return count <= 1;
}
static bool check_args_pair_invalid(enum bpf_arg_type arg_curr,
enum bpf_arg_type arg_next)
{
return (arg_type_is_mem_ptr(arg_curr) &&
!arg_type_is_mem_size(arg_next)) ||
(!arg_type_is_mem_ptr(arg_curr) &&
arg_type_is_mem_size(arg_next));
}
static bool check_arg_pair_ok(const struct bpf_func_proto *fn)
{
/* bpf_xxx(..., buf, len) call will access 'len'
* bytes from memory 'buf'. Both arg types need
* to be paired, so make sure there's no buggy
* helper function specification.
*/
if (arg_type_is_mem_size(fn->arg1_type) ||
arg_type_is_mem_ptr(fn->arg5_type) ||
check_args_pair_invalid(fn->arg1_type, fn->arg2_type) ||
check_args_pair_invalid(fn->arg2_type, fn->arg3_type) ||
check_args_pair_invalid(fn->arg3_type, fn->arg4_type) ||
check_args_pair_invalid(fn->arg4_type, fn->arg5_type))
return false;
return true;
}
static int check_func_proto(const struct bpf_func_proto *fn)
{
return check_raw_mode_ok(fn) &&
check_arg_pair_ok(fn) ? 0 : -EINVAL;
} }
/* Packet data might have moved, any old PTR_TO_PACKET[_META,_END] /* Packet data might have moved, any old PTR_TO_PACKET[_META,_END]
...@@ -2282,7 +2317,6 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn ...@@ -2282,7 +2317,6 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
if (env->ops->get_func_proto) if (env->ops->get_func_proto)
fn = env->ops->get_func_proto(func_id); fn = env->ops->get_func_proto(func_id);
if (!fn) { if (!fn) {
verbose(env, "unknown func %s#%d\n", func_id_name(func_id), verbose(env, "unknown func %s#%d\n", func_id_name(func_id),
func_id); func_id);
...@@ -2306,10 +2340,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn ...@@ -2306,10 +2340,7 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
memset(&meta, 0, sizeof(meta)); memset(&meta, 0, sizeof(meta));
meta.pkt_access = fn->pkt_access; meta.pkt_access = fn->pkt_access;
/* We only support one arg being in raw mode at the moment, which err = check_func_proto(fn);
* is sufficient for the helper functions we have right now.
*/
err = check_raw_mode(fn);
if (err) { if (err) {
verbose(env, "kernel subsystem misconfigured func %s#%d\n", verbose(env, "kernel subsystem misconfigured func %s#%d\n",
func_id_name(func_id), func_id); func_id_name(func_id), func_id);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment