Commit fe9a5ca7 authored by Daniel Borkmann's avatar Daniel Borkmann

bpf: Do not mark insn as seen under speculative path verification

... 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: default avatarDaniel Borkmann <daniel@iogearbox.net>
Reviewed-by: default avatarJohn Fastabend <john.fastabend@gmail.com>
Reviewed-by: default avatarBenedict Schlueter <benedict.schlueter@rub.de>
Reviewed-by: default avatarPiotr Krysiuk <piotras@gmail.com>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent d203b0fd
...@@ -6572,6 +6572,19 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env, ...@@ -6572,6 +6572,19 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
return !ret ? REASON_STACK : 0; 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, static int sanitize_err(struct bpf_verifier_env *env,
const struct bpf_insn *insn, int reason, const struct bpf_insn *insn, int reason,
const struct bpf_reg_state *off_reg, const struct bpf_reg_state *off_reg,
...@@ -10630,7 +10643,7 @@ static int do_check(struct bpf_verifier_env *env) ...@@ -10630,7 +10643,7 @@ static int do_check(struct bpf_verifier_env *env)
} }
regs = cur_regs(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; prev_insn_idx = env->insn_idx;
if (class == BPF_ALU || class == BPF_ALU64) { if (class == BPF_ALU || class == BPF_ALU64) {
...@@ -10857,7 +10870,7 @@ static int do_check(struct bpf_verifier_env *env) ...@@ -10857,7 +10870,7 @@ static int do_check(struct bpf_verifier_env *env)
return err; return err;
env->insn_idx++; env->insn_idx++;
env->insn_aux_data[env->insn_idx].seen = env->pass_cnt; sanitize_mark_insn_seen(env);
} else { } else {
verbose(env, "invalid BPF_LD mode\n"); verbose(env, "invalid BPF_LD mode\n");
return -EINVAL; return -EINVAL;
...@@ -12712,6 +12725,9 @@ static void free_states(struct bpf_verifier_env *env) ...@@ -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 * 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 * data from failed pass. For testing and experiments do_check_common() can be
* run multiple times even when prior attempt to verify is unsuccessful. * 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) static void sanitize_insn_aux_data(struct bpf_verifier_env *env)
{ {
......
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