Skip to content

Commit

Permalink
bpf: enforce exact retval range on subprog/callback exit
Browse files Browse the repository at this point in the history 
Instead of relying on potentially imprecise tnum representation of
expected return value range for callbacks and subprogs, validate that
smin/smax range satisfy exact expected range of return values.

E.g., if callback would need to return [0, 2] range, tnum can't
represent this precisely and instead will allow [0, 3] range. By
checking smin/smax range, we can make sure that subprog/callback indeed
returns only valid [0, 2] range.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231202175705.885270-5-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Andrii Nakryiko authored and Alexei Starovoitov committed Dec 2, 2023
1 parent 0acd03a commit 8fa4ecd
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 12 deletions.
7 changes: 6 additions & 1 deletion include/linux/bpf_verifier.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ struct bpf_reference_state {
int callback_ref;
};

struct bpf_retval_range {
s32 minval;
s32 maxval;
};

/* state of the program:
* type of all registers and stack info
*/
Expand All @@ -297,7 +302,7 @@ struct bpf_func_state {
* void foo(void) { bpf_timer_set_callback(,foo); }
*/
u32 async_entry_cnt;
struct tnum callback_ret_range;
struct bpf_retval_range callback_ret_range;
bool in_callback_fn;
bool in_async_callback_fn;
bool in_exception_callback_fn;
Expand Down
33 changes: 22 additions & 11 deletions kernel/bpf/verifier.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2305,6 +2305,11 @@ static void init_reg_state(struct bpf_verifier_env *env,
regs[BPF_REG_FP].frameno = state->frameno;
}

static struct bpf_retval_range retval_range(s32 minval, s32 maxval)
{
return (struct bpf_retval_range){ minval, maxval };
}

#define BPF_MAIN_FUNC (-1)
static void init_func_state(struct bpf_verifier_env *env,
struct bpf_func_state *state,
Expand All @@ -2313,7 +2318,7 @@ static void init_func_state(struct bpf_verifier_env *env,
state->callsite = callsite;
state->frameno = frameno;
state->subprogno = subprogno;
state->callback_ret_range = tnum_range(0, 0);
state->callback_ret_range = retval_range(0, 0);
init_reg_state(env, state);
mark_verifier_state_scratched(env);
}
Expand Down Expand Up @@ -9396,7 +9401,7 @@ static int set_map_elem_callback_state(struct bpf_verifier_env *env,
return err;

callee->in_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand All @@ -9418,7 +9423,7 @@ static int set_loop_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);

callee->in_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand Down Expand Up @@ -9448,7 +9453,7 @@ static int set_timer_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);
callee->in_async_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand Down Expand Up @@ -9476,7 +9481,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);
callee->in_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand All @@ -9499,7 +9504,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);

callee->in_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand Down Expand Up @@ -9531,7 +9536,7 @@ static int set_rbtree_add_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_4]);
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);
callee->in_callback_fn = true;
callee->callback_ret_range = tnum_range(0, 1);
callee->callback_ret_range = retval_range(0, 1);
return 0;
}

Expand Down Expand Up @@ -9560,6 +9565,11 @@ static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env)
return is_rbtree_lock_required_kfunc(kfunc_btf_id);
}

static bool retval_range_within(struct bpf_retval_range range, const struct bpf_reg_state *reg)
{
return range.minval <= reg->smin_value && reg->smax_value <= range.maxval;
}

static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
{
struct bpf_verifier_state *state = env->cur_state, *prev_st;
Expand All @@ -9583,9 +9593,6 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)

caller = state->frame[state->curframe - 1];
if (callee->in_callback_fn) {
/* enforce R0 return value range [0, 1]. */
struct tnum range = callee->callback_ret_range;

if (r0->type != SCALAR_VALUE) {
verbose(env, "R0 not a scalar value\n");
return -EACCES;
Expand All @@ -9597,7 +9604,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
if (err)
return err;

if (!tnum_in(range, r0->var_off)) {
/* enforce R0 return value range */
if (!retval_range_within(callee->callback_ret_range, r0)) {
struct tnum range = tnum_range(callee->callback_ret_range.minval,
callee->callback_ret_range.maxval);

verbose_invalid_scalar(env, r0, &range, "callback return", "R0");
return -EINVAL;
}
Expand Down

0 comments on commit 8fa4ecd

Please sign in to comment.