Skip to content

Commit

Permalink
Merge branch 'bpf-verifier-improvements'
Browse files Browse the repository at this point in the history 
Alexei Starovoitov says:

====================
bpf: verifier improvements

A number of bpf verifier improvements from Gianluca.
See individual patches for details.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jan 9, 2017
2 parents f3a3e24 + 39f19eb commit c22e5c1
Show file tree
Hide file tree
Showing 6 changed files with 1,131 additions and 104 deletions.
12 changes: 6 additions & 6 deletions include/linux/bpf.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ enum bpf_arg_type {
/* the following constraints used to prototype bpf_memcmp() and other
* functions that access data on eBPF program stack
*/
ARG_PTR_TO_STACK, /* any pointer to eBPF program stack */
ARG_PTR_TO_RAW_STACK, /* any pointer to eBPF program stack, area does not
* need to be initialized, helper function must fill
* all bytes or clear them in error case.
ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value) */
ARG_PTR_TO_UNINIT_MEM, /* pointer to memory does not need to be initialized,
* helper function must fill all bytes or clear
* them in error case.
*/

ARG_CONST_STACK_SIZE, /* number of bytes accessed from stack */
ARG_CONST_STACK_SIZE_OR_ZERO, /* number of bytes accessed from stack or 0 */
ARG_CONST_SIZE, /* number of bytes accessed from memory */
ARG_CONST_SIZE_OR_ZERO, /* number of bytes accessed from memory or 0 */

ARG_PTR_TO_CTX, /* pointer to context */
ARG_ANYTHING, /* any (initialized) argument is ok */
Expand Down
4 changes: 2 additions & 2 deletions kernel/bpf/helpers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,6 @@ const struct bpf_func_proto bpf_get_current_comm_proto = {
.func = bpf_get_current_comm,
.gpl_only = false,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_RAW_STACK,
.arg2_type = ARG_CONST_STACK_SIZE,
.arg1_type = ARG_PTR_TO_UNINIT_MEM,
.arg2_type = ARG_CONST_SIZE,
};
212 changes: 146 additions & 66 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,13 @@ static void reset_reg_range_values(struct bpf_reg_state *regs, u32 regno)
regs[regno].max_value = BPF_REGISTER_MAX_RANGE;
}

static void mark_reg_unknown_value_and_range(struct bpf_reg_state *regs,
u32 regno)
{
mark_reg_unknown_value(regs, regno);
reset_reg_range_values(regs, regno);
}

enum reg_arg_type {
SRC_OP, /* register is used as source operand */
DST_OP, /* register is used as destination operand */
Expand Down Expand Up @@ -532,6 +539,7 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
switch (type) {
case PTR_TO_MAP_VALUE:
case PTR_TO_MAP_VALUE_OR_NULL:
case PTR_TO_MAP_VALUE_ADJ:
case PTR_TO_STACK:
case PTR_TO_CTX:
case PTR_TO_PACKET:
Expand Down Expand Up @@ -616,7 +624,8 @@ static int check_stack_read(struct bpf_verifier_state *state, int off, int size,
}
if (value_regno >= 0)
/* have read misc data from the stack */
mark_reg_unknown_value(state->regs, value_regno);
mark_reg_unknown_value_and_range(state->regs,
value_regno);
return 0;
}
}
Expand All @@ -627,14 +636,59 @@ static int check_map_access(struct bpf_verifier_env *env, u32 regno, int off,
{
struct bpf_map *map = env->cur_state.regs[regno].map_ptr;

if (off < 0 || off + size > map->value_size) {
if (off < 0 || size <= 0 || off + size > map->value_size) {
verbose("invalid access to map value, value_size=%d off=%d size=%d\n",
map->value_size, off, size);
return -EACCES;
}
return 0;
}

/* check read/write into an adjusted map element */
static int check_map_access_adj(struct bpf_verifier_env *env, u32 regno,
int off, int size)
{
struct bpf_verifier_state *state = &env->cur_state;
struct bpf_reg_state *reg = &state->regs[regno];
int err;

/* We adjusted the register to this map value, so we
* need to change off and size to min_value and max_value
* respectively to make sure our theoretical access will be
* safe.
*/
if (log_level)
print_verifier_state(state);
env->varlen_map_value_access = true;
/* The minimum value is only important with signed
* comparisons where we can't assume the floor of a
* value is 0. If we are using signed variables for our
* index'es we need to make sure that whatever we use
* will have a set floor within our range.
*/
if (reg->min_value < 0) {
verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
regno);
return -EACCES;
}
err = check_map_access(env, regno, reg->min_value + off, size);
if (err) {
verbose("R%d min value is outside of the array range\n",
regno);
return err;
}

/* If we haven't set a max value then we need to bail
* since we can't be sure we won't do bad things.
*/
if (reg->max_value == BPF_REGISTER_MAX_RANGE) {
verbose("R%d unbounded memory access, make sure to bounds check any array access into a map\n",
regno);
return -EACCES;
}
return check_map_access(env, regno, reg->max_value + off, size);
}

#define MAX_PACKET_OFF 0xffff

static bool may_access_direct_pkt_data(struct bpf_verifier_env *env,
Expand Down Expand Up @@ -775,47 +829,13 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
return -EACCES;
}

/* If we adjusted the register to this map value at all then we
* need to change off and size to min_value and max_value
* respectively to make sure our theoretical access will be
* safe.
*/
if (reg->type == PTR_TO_MAP_VALUE_ADJ) {
if (log_level)
print_verifier_state(state);
env->varlen_map_value_access = true;
/* The minimum value is only important with signed
* comparisons where we can't assume the floor of a
* value is 0. If we are using signed variables for our
* index'es we need to make sure that whatever we use
* will have a set floor within our range.
*/
if (reg->min_value < 0) {
verbose("R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
regno);
return -EACCES;
}
err = check_map_access(env, regno, reg->min_value + off,
size);
if (err) {
verbose("R%d min value is outside of the array range\n",
regno);
return err;
}

/* If we haven't set a max value then we need to bail
* since we can't be sure we won't do bad things.
*/
if (reg->max_value == BPF_REGISTER_MAX_RANGE) {
verbose("R%d unbounded memory access, make sure to bounds check any array access into a map\n",
regno);
return -EACCES;
}
off += reg->max_value;
}
err = check_map_access(env, regno, off, size);
if (reg->type == PTR_TO_MAP_VALUE_ADJ)
err = check_map_access_adj(env, regno, off, size);
else
err = check_map_access(env, regno, off, size);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown_value(state->regs, value_regno);
mark_reg_unknown_value_and_range(state->regs,
value_regno);

} else if (reg->type == PTR_TO_CTX) {
enum bpf_reg_type reg_type = UNKNOWN_VALUE;
Expand All @@ -827,7 +847,8 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
}
err = check_ctx_access(env, off, size, t, &reg_type);
if (!err && t == BPF_READ && value_regno >= 0) {
mark_reg_unknown_value(state->regs, value_regno);
mark_reg_unknown_value_and_range(state->regs,
value_regno);
/* note that reg.[id|off|range] == 0 */
state->regs[value_regno].type = reg_type;
}
Expand Down Expand Up @@ -860,7 +881,8 @@ static int check_mem_access(struct bpf_verifier_env *env, u32 regno, int off,
}
err = check_packet_access(env, regno, off, size);
if (!err && t == BPF_READ && value_regno >= 0)
mark_reg_unknown_value(state->regs, value_regno);
mark_reg_unknown_value_and_range(state->regs,
value_regno);
} else {
verbose("R%d invalid mem access '%s'\n",
regno, reg_type_str[reg->type]);
Expand Down Expand Up @@ -958,6 +980,25 @@ static int check_stack_boundary(struct bpf_verifier_env *env, int regno,
return 0;
}

static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
int access_size, bool zero_size_allowed,
struct bpf_call_arg_meta *meta)
{
struct bpf_reg_state *regs = env->cur_state.regs;

switch (regs[regno].type) {
case PTR_TO_PACKET:
return check_packet_access(env, regno, 0, access_size);
case PTR_TO_MAP_VALUE:
return check_map_access(env, regno, 0, access_size);
case PTR_TO_MAP_VALUE_ADJ:
return check_map_access_adj(env, regno, 0, access_size);
default: /* const_imm|ptr_to_stack or invalid ptr */
return check_stack_boundary(env, regno, access_size,
zero_size_allowed, meta);
}
}

static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
enum bpf_arg_type arg_type,
struct bpf_call_arg_meta *meta)
Expand Down Expand Up @@ -993,10 +1034,13 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
expected_type = PTR_TO_STACK;
if (type != PTR_TO_PACKET && type != expected_type)
goto err_type;
} else if (arg_type == ARG_CONST_STACK_SIZE ||
arg_type == ARG_CONST_STACK_SIZE_OR_ZERO) {
} else if (arg_type == ARG_CONST_SIZE ||
arg_type == ARG_CONST_SIZE_OR_ZERO) {
expected_type = CONST_IMM;
if (type != expected_type)
/* One exception. Allow UNKNOWN_VALUE registers when the
* boundaries are known and don't cause unsafe memory accesses
*/
if (type != UNKNOWN_VALUE && type != expected_type)
goto err_type;
} else if (arg_type == ARG_CONST_MAP_PTR) {
expected_type = CONST_PTR_TO_MAP;
Expand All @@ -1006,18 +1050,19 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
expected_type = PTR_TO_CTX;
if (type != expected_type)
goto err_type;
} else if (arg_type == ARG_PTR_TO_STACK ||
arg_type == ARG_PTR_TO_RAW_STACK) {
} else if (arg_type == ARG_PTR_TO_MEM ||
arg_type == ARG_PTR_TO_UNINIT_MEM) {
expected_type = PTR_TO_STACK;
/* One exception here. In case function allows for NULL to be
* passed in as argument, it's a CONST_IMM type. Final test
* happens during stack boundary checking.
*/
if (type == CONST_IMM && reg->imm == 0)
/* final test in check_stack_boundary() */;
else if (type != PTR_TO_PACKET && type != expected_type)
else if (type != PTR_TO_PACKET && type != PTR_TO_MAP_VALUE &&
type != PTR_TO_MAP_VALUE_ADJ && type != expected_type)
goto err_type;
meta->raw_mode = arg_type == ARG_PTR_TO_RAW_STACK;
meta->raw_mode = arg_type == ARG_PTR_TO_UNINIT_MEM;
} else {
verbose("unsupported arg_type %d\n", arg_type);
return -EFAULT;
Expand Down Expand Up @@ -1063,24 +1108,60 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
err = check_stack_boundary(env, regno,
meta->map_ptr->value_size,
false, NULL);
} else if (arg_type == ARG_CONST_STACK_SIZE ||
arg_type == ARG_CONST_STACK_SIZE_OR_ZERO) {
bool zero_size_allowed = (arg_type == ARG_CONST_STACK_SIZE_OR_ZERO);
} else if (arg_type == ARG_CONST_SIZE ||
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("ARG_CONST_STACK_SIZE cannot be first argument\n");
verbose("ARG_CONST_SIZE cannot be first argument\n");
return -EACCES;
}
if (regs[regno - 1].type == PTR_TO_PACKET)
err = check_packet_access(env, regno - 1, 0, reg->imm);
else
err = check_stack_boundary(env, regno - 1, reg->imm,
zero_size_allowed, meta);

/* If the register is UNKNOWN_VALUE, the access check happens
* using its boundaries. Otherwise, just use its imm
*/
if (type == UNKNOWN_VALUE) {
/* For unprivileged variable accesses, disable raw
* mode so that the program is required to
* initialize all the memory that the helper could
* just partially fill up.
*/
meta = NULL;

if (reg->min_value < 0) {
verbose("R%d min value is negative, either use unsigned or 'var &= const'\n",
regno);
return -EACCES;
}

if (reg->min_value == 0) {
err = check_helper_mem_access(env, regno - 1, 0,
zero_size_allowed,
meta);
if (err)
return err;
}

if (reg->max_value == BPF_REGISTER_MAX_RANGE) {
verbose("R%d unbounded memory access, use 'var &= const' or 'if (var < const)'\n",
regno);
return -EACCES;
}
err = check_helper_mem_access(env, regno - 1,
reg->max_value,
zero_size_allowed, meta);
if (err)
return err;
} else {
/* register is CONST_IMM */
err = check_helper_mem_access(env, regno - 1, reg->imm,
zero_size_allowed, meta);
}
}

return err;
Expand Down Expand Up @@ -1154,15 +1235,15 @@ static int check_raw_mode(const struct bpf_func_proto *fn)
{
int count = 0;

if (fn->arg1_type == ARG_PTR_TO_RAW_STACK)
if (fn->arg1_type == ARG_PTR_TO_UNINIT_MEM)
count++;
if (fn->arg2_type == ARG_PTR_TO_RAW_STACK)
if (fn->arg2_type == ARG_PTR_TO_UNINIT_MEM)
count++;
if (fn->arg3_type == ARG_PTR_TO_RAW_STACK)
if (fn->arg3_type == ARG_PTR_TO_UNINIT_MEM)
count++;
if (fn->arg4_type == ARG_PTR_TO_RAW_STACK)
if (fn->arg4_type == ARG_PTR_TO_UNINIT_MEM)
count++;
if (fn->arg5_type == ARG_PTR_TO_RAW_STACK)
if (fn->arg5_type == ARG_PTR_TO_UNINIT_MEM)
count++;

return count > 1 ? -EINVAL : 0;
Expand Down Expand Up @@ -2729,7 +2810,6 @@ static int do_check(struct bpf_verifier_env *env)
if (err)
return err;

reset_reg_range_values(regs, insn->dst_reg);
if (BPF_SIZE(insn->code) != BPF_W &&
BPF_SIZE(insn->code) != BPF_DW) {
insn_idx++;
Expand Down
Loading

0 comments on commit c22e5c1

Please sign in to comment.