Skip to content

Commit b74c2a8

Browse files
anakryikoAlexei Starovoitov
authored andcommitted
bpf: generalize is_branch_taken to handle all conditional jumps in one place
Make is_branch_taken() a single entry point for branch pruning decision making, handling both pointer vs pointer, pointer vs scalar, and scalar vs scalar cases in one place. This also nicely cleans up check_cond_jmp_op(). Acked-by: Eduard Zingerman <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent c697289 commit b74c2a8

File tree

1 file changed

+25
-24
lines changed

1 file changed

+25
-24
lines changed

kernel/bpf/verifier.c

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14188,6 +14188,19 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate,
1418814188
}));
1418914189
}
1419014190

14191+
/* check if register is a constant scalar value */
14192+
static bool is_reg_const(struct bpf_reg_state *reg, bool subreg32)
14193+
{
14194+
return reg->type == SCALAR_VALUE &&
14195+
tnum_is_const(subreg32 ? tnum_subreg(reg->var_off) : reg->var_off);
14196+
}
14197+
14198+
/* assuming is_reg_const() is true, return constant value of a register */
14199+
static u64 reg_const_value(struct bpf_reg_state *reg, bool subreg32)
14200+
{
14201+
return subreg32 ? tnum_subreg(reg->var_off).value : reg->var_off.value;
14202+
}
14203+
1419114204
/*
1419214205
* <reg1> <op> <reg2>, currently assuming reg2 is a constant
1419314206
*/
@@ -14429,12 +14442,20 @@ static int is_pkt_ptr_branch_taken(struct bpf_reg_state *dst_reg,
1442914442
static int is_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_state *reg2,
1443014443
u8 opcode, bool is_jmp32)
1443114444
{
14432-
struct tnum reg2_tnum = is_jmp32 ? tnum_subreg(reg2->var_off) : reg2->var_off;
1443314445
u64 val;
1443414446

14435-
if (!tnum_is_const(reg2_tnum))
14447+
if (reg_is_pkt_pointer_any(reg1) && reg_is_pkt_pointer_any(reg2) && !is_jmp32)
14448+
return is_pkt_ptr_branch_taken(reg1, reg2, opcode);
14449+
14450+
/* try to make sure reg2 is a constant SCALAR_VALUE */
14451+
if (!is_reg_const(reg2, is_jmp32)) {
14452+
opcode = flip_opcode(opcode);
14453+
swap(reg1, reg2);
14454+
}
14455+
/* for now we expect reg2 to be a constant to make any useful decisions */
14456+
if (!is_reg_const(reg2, is_jmp32))
1443614457
return -1;
14437-
val = reg2_tnum.value;
14458+
val = reg_const_value(reg2, is_jmp32);
1443814459

1443914460
if (__is_pointer_value(false, reg1)) {
1444014461
if (!reg_not_null(reg1))
@@ -14915,27 +14936,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
1491514936
}
1491614937

1491714938
is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32;
14918-
14919-
if (BPF_SRC(insn->code) == BPF_K) {
14920-
pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
14921-
} else if (src_reg->type == SCALAR_VALUE &&
14922-
is_jmp32 && tnum_is_const(tnum_subreg(src_reg->var_off))) {
14923-
pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
14924-
} else if (src_reg->type == SCALAR_VALUE &&
14925-
!is_jmp32 && tnum_is_const(src_reg->var_off)) {
14926-
pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
14927-
} else if (dst_reg->type == SCALAR_VALUE &&
14928-
is_jmp32 && tnum_is_const(tnum_subreg(dst_reg->var_off))) {
14929-
pred = is_branch_taken(src_reg, dst_reg, flip_opcode(opcode), is_jmp32);
14930-
} else if (dst_reg->type == SCALAR_VALUE &&
14931-
!is_jmp32 && tnum_is_const(dst_reg->var_off)) {
14932-
pred = is_branch_taken(src_reg, dst_reg, flip_opcode(opcode), is_jmp32);
14933-
} else if (reg_is_pkt_pointer_any(dst_reg) &&
14934-
reg_is_pkt_pointer_any(src_reg) &&
14935-
!is_jmp32) {
14936-
pred = is_pkt_ptr_branch_taken(dst_reg, src_reg, opcode);
14937-
}
14938-
14939+
pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
1493914940
if (pred >= 0) {
1494014941
/* If we get here with a dst_reg pointer type it is because
1494114942
* above is_branch_taken() special cased the 0 comparison.

0 commit comments

Comments
 (0)