Commit 8fe2d6cc authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by David S. Miller

bpf: fix liveness marking

while processing Rx = Ry instruction the verifier does
regs[insn->dst_reg] = regs[insn->src_reg]
which often clears write mark (when Ry doesn't have it)
that was just set by check_reg_arg(Rx) prior to the assignment.
That causes mark_reg_read() to keep marking Rx in this block as
REG_LIVE_READ (since the logic incorrectly misses that it's
screened by the write) and in many of its parents (until lucky
write into the same Rx or beginning of the program).
That causes is_state_visited() logic to miss many pruning opportunities.

Furthermore mark_reg_read() logic propagates the read mark
for BPF_REG_FP as well (though it's readonly) which causes
harmless but unnecssary work during is_state_visited().
Note that do_propagate_liveness() skips FP correctly,
so do the same in mark_reg_read() as well.
It saves 0.2 seconds for the test below

program               before  after
bpf_lb-DLB_L3.o       2604    2304
bpf_lb-DLB_L4.o       11159   3723
bpf_lb-DUNKNOWN.o     1116    1110
bpf_lxc-DDROP_ALL.o   34566   28004
bpf_lxc-DUNKNOWN.o    53267   39026
bpf_netdev.o          17843   16943
bpf_overlay.o         8672    7929
time                  ~11 sec  ~4 sec

Fixes: dc503a8a ("bpf/verifier: track liveness for pruning")
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarEdward Cree <ecree@solarflare.com>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 00a534e5
...@@ -653,6 +653,10 @@ static void mark_reg_read(const struct bpf_verifier_state *state, u32 regno) ...@@ -653,6 +653,10 @@ static void mark_reg_read(const struct bpf_verifier_state *state, u32 regno)
{ {
struct bpf_verifier_state *parent = state->parent; struct bpf_verifier_state *parent = state->parent;
if (regno == BPF_REG_FP)
/* We don't need to worry about FP liveness because it's read-only */
return;
while (parent) { while (parent) {
/* if read wasn't screened by an earlier write ... */ /* if read wasn't screened by an earlier write ... */
if (state->regs[regno].live & REG_LIVE_WRITTEN) if (state->regs[regno].live & REG_LIVE_WRITTEN)
...@@ -2345,6 +2349,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn) ...@@ -2345,6 +2349,7 @@ static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
* copy register state to dest reg * copy register state to dest reg
*/ */
regs[insn->dst_reg] = regs[insn->src_reg]; regs[insn->dst_reg] = regs[insn->src_reg];
regs[insn->dst_reg].live |= REG_LIVE_WRITTEN;
} else { } else {
/* R1 = (u32) R2 */ /* R1 = (u32) R2 */
if (is_pointer_value(env, insn->src_reg)) { if (is_pointer_value(env, insn->src_reg)) {
......
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