Skip to content

Commit

Permalink
bpf: Do not mark insn as seen under speculative path verification
Browse files Browse the repository at this point in the history
... in such circumstances, we do not want to mark the instruction as seen given
the goal is still to jmp-1 rewrite/sanitize dead code, if it is not reachable
from the non-speculative path verification. We do however want to verify it for
safety regardless.

With the patch as-is all the insns that have been marked as seen before the
patch will also be marked as seen after the patch (just with a potentially
different non-zero count). An upcoming patch will also verify paths that are
unreachable in the non-speculative domain, hence this extension is needed.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Reviewed-by: Benedict Schlueter <benedict.schlueter@rub.de>
Reviewed-by: Piotr Krysiuk <piotras@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Daniel Borkmann committed Jun 14, 2021
1 parent d203b0f commit fe9a5ca
Showing 1 changed file with 18 additions and 2 deletions.
20 changes: 18 additions & 2 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -6572,6 +6572,19 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
return !ret ? REASON_STACK : 0;
}

static void sanitize_mark_insn_seen(struct bpf_verifier_env *env)
{
struct bpf_verifier_state *vstate = env->cur_state;

/* If we simulate paths under speculation, we don't update the
* insn as 'seen' such that when we verify unreachable paths in
* the non-speculative domain, sanitize_dead_code() can still
* rewrite/sanitize them.
*/
if (!vstate->speculative)
env->insn_aux_data[env->insn_idx].seen = env->pass_cnt;
}

static int sanitize_err(struct bpf_verifier_env *env,
const struct bpf_insn *insn, int reason,
const struct bpf_reg_state *off_reg,
Expand Down Expand Up @@ -10630,7 +10643,7 @@ static int do_check(struct bpf_verifier_env *env)
}

regs = cur_regs(env);
env->insn_aux_data[env->insn_idx].seen = env->pass_cnt;
sanitize_mark_insn_seen(env);
prev_insn_idx = env->insn_idx;

if (class == BPF_ALU || class == BPF_ALU64) {
Expand Down Expand Up @@ -10857,7 +10870,7 @@ static int do_check(struct bpf_verifier_env *env)
return err;

env->insn_idx++;
env->insn_aux_data[env->insn_idx].seen = env->pass_cnt;
sanitize_mark_insn_seen(env);
} else {
verbose(env, "invalid BPF_LD mode\n");
return -EINVAL;
Expand Down Expand Up @@ -12712,6 +12725,9 @@ static void free_states(struct bpf_verifier_env *env)
* insn_aux_data was touched. These variables are compared to clear temporary
* data from failed pass. For testing and experiments do_check_common() can be
* run multiple times even when prior attempt to verify is unsuccessful.
*
* Note that special handling is needed on !env->bypass_spec_v1 if this is
* ever called outside of error path with subsequent program rejection.
*/
static void sanitize_insn_aux_data(struct bpf_verifier_env *env)
{
Expand Down

0 comments on commit fe9a5ca

Please sign in to comment.