Skip to content

Commit

Permalink
bpf: Improve JEQ/JNE branch taken logic
Browse files Browse the repository at this point in the history
When determining if an if/else branch will always or never be taken, use
signed range knowledge in addition to currently used unsigned range knowledge.
If either signed or unsigned range suggests that condition is always/never
taken, return corresponding branch_taken verdict.

Current use of unsigned range for this seems arbitrary and unnecessarily
incomplete. It is possible for *signed* operations to be performed on
register, which could "invalidate" unsigned range for that register. In such
case branch_taken will be artificially useless, even if we can still tell
that some constant is outside of register value range based on its signed
bounds.

veristat-based validation shows zero differences across selftests, Cilium,
and Meta-internal BPF object files.

Signed-off-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Acked-by: Shung-Hsi Yu <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
  • Loading branch information
anakryiko authored and borkmann committed Oct 24, 2023
1 parent 06646da commit 42d31dd
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -14031,12 +14031,16 @@ static int is_branch32_taken(struct bpf_reg_state *reg, u32 val, u8 opcode)
return !!tnum_equals_const(subreg, val);
else if (val < reg->u32_min_value || val > reg->u32_max_value)
return 0;
else if (sval < reg->s32_min_value || sval > reg->s32_max_value)
return 0;
break;
case BPF_JNE:
if (tnum_is_const(subreg))
return !tnum_equals_const(subreg, val);
else if (val < reg->u32_min_value || val > reg->u32_max_value)
return 1;
else if (sval < reg->s32_min_value || sval > reg->s32_max_value)
return 1;
break;
case BPF_JSET:
if ((~subreg.mask & subreg.value) & val)
Expand Down Expand Up @@ -14108,12 +14112,16 @@ static int is_branch64_taken(struct bpf_reg_state *reg, u64 val, u8 opcode)
return !!tnum_equals_const(reg->var_off, val);
else if (val < reg->umin_value || val > reg->umax_value)
return 0;
else if (sval < reg->smin_value || sval > reg->smax_value)
return 0;
break;
case BPF_JNE:
if (tnum_is_const(reg->var_off))
return !tnum_equals_const(reg->var_off, val);
else if (val < reg->umin_value || val > reg->umax_value)
return 1;
else if (sval < reg->smin_value || sval > reg->smax_value)
return 1;
break;
case BPF_JSET:
if ((~reg->var_off.mask & reg->var_off.value) & val)
Expand Down

0 comments on commit 42d31dd

Please sign in to comment.