bpf: improve dead code sanitizing
Given we recently hadc131187db2
("bpf: fix branch pruning logic") and95a762e2c8
("bpf: fix incorrect sign extension in check_alu_op()") in particular where before verifier skipped verification of the wrongly assumed dead branch, we should not just replace the dead code parts with nops (mov r0,r0). If there is a bug such as fixed in95a762e2c8
in future again, where runtime could execute those insns, then one of the potential issues with the current setting would be that given the nops would be at the end of the program, we could execute out of bounds at some point. The best in such case would be to just exit the BPF program altogether and return an exception code. However, given this would require two instructions, and such a dead code gap could just be a single insn long, we would need to place 'r0 = X; ret' snippet at the very end after the user program or at the start before the program (where we'd skip that region on prog entry), and then place unconditional ja's into the dead code gap. While more complex but possible, there's still another block in the road that currently prevents from this, namely BPF to BPF calls. The issue here is that such exception could be returned from a callee, but the caller would not know that it's an exception that needs to be propagated further down. Alternative that has little complexity is to just use a ja-1 code for now which will trap the execution here instead of silently doing bad things if we ever get there due to bugs. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
parent
1d621674d9
commit
2a5418a13f
|
@ -5064,14 +5064,21 @@ static struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 of
|
||||||
return new_prog;
|
return new_prog;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* The verifier does more data flow analysis than llvm and will not explore
|
/* The verifier does more data flow analysis than llvm and will not
|
||||||
* branches that are dead at run time. Malicious programs can have dead code
|
* explore branches that are dead at run time. Malicious programs can
|
||||||
* too. Therefore replace all dead at-run-time code with nops.
|
* have dead code too. Therefore replace all dead at-run-time code
|
||||||
|
* with 'ja -1'.
|
||||||
|
*
|
||||||
|
* Just nops are not optimal, e.g. if they would sit at the end of the
|
||||||
|
* program and through another bug we would manage to jump there, then
|
||||||
|
* we'd execute beyond program memory otherwise. Returning exception
|
||||||
|
* code also wouldn't work since we can have subprogs where the dead
|
||||||
|
* code could be located.
|
||||||
*/
|
*/
|
||||||
static void sanitize_dead_code(struct bpf_verifier_env *env)
|
static void sanitize_dead_code(struct bpf_verifier_env *env)
|
||||||
{
|
{
|
||||||
struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
|
struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
|
||||||
struct bpf_insn nop = BPF_MOV64_REG(BPF_REG_0, BPF_REG_0);
|
struct bpf_insn trap = BPF_JMP_IMM(BPF_JA, 0, 0, -1);
|
||||||
struct bpf_insn *insn = env->prog->insnsi;
|
struct bpf_insn *insn = env->prog->insnsi;
|
||||||
const int insn_cnt = env->prog->len;
|
const int insn_cnt = env->prog->len;
|
||||||
int i;
|
int i;
|
||||||
|
@ -5079,7 +5086,7 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
|
||||||
for (i = 0; i < insn_cnt; i++) {
|
for (i = 0; i < insn_cnt; i++) {
|
||||||
if (aux_data[i].seen)
|
if (aux_data[i].seen)
|
||||||
continue;
|
continue;
|
||||||
memcpy(insn + i, &nop, sizeof(nop));
|
memcpy(insn + i, &trap, sizeof(trap));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue