Skip to content

Commit

Permalink
bpf: Add verifier support for custom callback return range
Browse files Browse the repository at this point in the history
Verifier logic to confirm that a callback function returns 0 or 1 was
added in commit 69c087b ("bpf: Add bpf_for_each_map_elem() helper").
At the time, callback return value was only used to continue or stop
iteration.

In order to support callbacks with a broader return value range, such as
those added in rbtree series[0] and others, add a callback_ret_range to
bpf_func_state. Verifier's helpers which set in_callback_fn will also
set the new field, which the verifier will later use to check return
value bounds.

Default to tnum_range(0, 0) instead of using tnum_unknown as a sentinel
value as the latter would prevent the valid range (0, U64_MAX) being
used. Previous global default tnum_range(0, 1) is explicitly set for
extant callback helpers. The change to global default was made after
discussion around this patch in rbtree series [1], goal here is to make
it more obvious that callback_ret_range should be explicitly set.

  [0]: lore.kernel.org/bpf/20220830172759.4069786-1-davemarchevsky@fb.com/
  [1]: lore.kernel.org/bpf/20220830172759.4069786-2-davemarchevsky@fb.com/

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Link: https://lore.kernel.org/r/20220908230716.2751723-1-davemarchevsky@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
davemarchevsky authored and Alexei Starovoitov committed Sep 11, 2022
1 parent f7c946f commit 1bfe26f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/linux/bpf_verifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ struct bpf_func_state {
*/
u32 async_entry_cnt;
bool in_callback_fn;
struct tnum callback_ret_range;
bool in_async_callback_fn;

/* The following fields should be last. See copy_func_state() */
Expand Down
7 changes: 6 additions & 1 deletion kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,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);
init_reg_state(env, state);
mark_verifier_state_scratched(env);
}
Expand Down Expand Up @@ -6790,6 +6791,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);
return 0;
}

Expand All @@ -6811,6 +6813,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);
return 0;
}

Expand Down Expand Up @@ -6840,6 +6843,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);
return 0;
}

Expand Down Expand Up @@ -6867,6 +6871,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);
return 0;
}

Expand Down Expand Up @@ -6894,7 +6899,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
caller = state->frame[state->curframe];
if (callee->in_callback_fn) {
/* enforce R0 return value range [0, 1]. */
struct tnum range = tnum_range(0, 1);
struct tnum range = callee->callback_ret_range;

if (r0->type != SCALAR_VALUE) {
verbose(env, "R0 not a scalar value\n");
Expand Down

0 comments on commit 1bfe26f

Please sign in to comment.