From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>,
<martin.lau@kernel.org>
Cc: <andrii@kernel.org>, <kernel-team@meta.com>,
Eduard Zingerman <eddyz87@gmail.com>
Subject: [PATCH v6 bpf-next 14/17] bpf: generalize is_branch_taken to handle all conditional jumps in one place
Date: Wed, 1 Nov 2023 20:37:56 -0700 [thread overview]
Message-ID: <20231102033759.2541186-15-andrii@kernel.org> (raw)
In-Reply-To: <20231102033759.2541186-1-andrii@kernel.org>
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 <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/verifier.c | 49 ++++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c5d187d43fa1..d5213cef5389 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14167,6 +14167,19 @@ static void find_good_pkt_pointers(struct bpf_verifier_state *vstate,
}));
}
+/* check if register is a constant scalar value */
+static bool is_reg_const(struct bpf_reg_state *reg, bool subreg32)
+{
+ return reg->type == SCALAR_VALUE &&
+ tnum_is_const(subreg32 ? tnum_subreg(reg->var_off) : reg->var_off);
+}
+
+/* assuming is_reg_const() is true, return constant value of a register */
+static u64 reg_const_value(struct bpf_reg_state *reg, bool subreg32)
+{
+ return subreg32 ? tnum_subreg(reg->var_off).value : reg->var_off.value;
+}
+
/*
* <reg1> <op> <reg2>, currently assuming reg2 is a constant
*/
@@ -14408,12 +14421,20 @@ static int is_pkt_ptr_branch_taken(struct bpf_reg_state *dst_reg,
static int is_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_state *reg2,
u8 opcode, bool is_jmp32)
{
- struct tnum reg2_tnum = is_jmp32 ? tnum_subreg(reg2->var_off) : reg2->var_off;
u64 val;
- if (!tnum_is_const(reg2_tnum))
+ if (reg_is_pkt_pointer_any(reg1) && reg_is_pkt_pointer_any(reg2) && !is_jmp32)
+ return is_pkt_ptr_branch_taken(reg1, reg2, opcode);
+
+ /* try to make sure reg2 is a constant SCALAR_VALUE */
+ if (!is_reg_const(reg2, is_jmp32)) {
+ opcode = flip_opcode(opcode);
+ swap(reg1, reg2);
+ }
+ /* for now we expect reg2 to be a constant to make any useful decisions */
+ if (!is_reg_const(reg2, is_jmp32))
return -1;
- val = reg2_tnum.value;
+ val = reg_const_value(reg2, is_jmp32);
if (__is_pointer_value(false, reg1)) {
if (!reg_not_null(reg1))
@@ -14894,27 +14915,7 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
}
is_jmp32 = BPF_CLASS(insn->code) == BPF_JMP32;
-
- if (BPF_SRC(insn->code) == BPF_K) {
- pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
- } else if (src_reg->type == SCALAR_VALUE &&
- is_jmp32 && tnum_is_const(tnum_subreg(src_reg->var_off))) {
- pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
- } else if (src_reg->type == SCALAR_VALUE &&
- !is_jmp32 && tnum_is_const(src_reg->var_off)) {
- pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
- } else if (dst_reg->type == SCALAR_VALUE &&
- is_jmp32 && tnum_is_const(tnum_subreg(dst_reg->var_off))) {
- pred = is_branch_taken(src_reg, dst_reg, flip_opcode(opcode), is_jmp32);
- } else if (dst_reg->type == SCALAR_VALUE &&
- !is_jmp32 && tnum_is_const(dst_reg->var_off)) {
- pred = is_branch_taken(src_reg, dst_reg, flip_opcode(opcode), is_jmp32);
- } else if (reg_is_pkt_pointer_any(dst_reg) &&
- reg_is_pkt_pointer_any(src_reg) &&
- !is_jmp32) {
- pred = is_pkt_ptr_branch_taken(dst_reg, src_reg, opcode);
- }
-
+ pred = is_branch_taken(dst_reg, src_reg, opcode, is_jmp32);
if (pred >= 0) {
/* If we get here with a dst_reg pointer type it is because
* above is_branch_taken() special cased the 0 comparison.
--
2.34.1
next prev parent reply other threads:[~2023-11-02 3:38 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-02 3:37 [PATCH v6 bpf-next 00/17] BPF register bounds logic and testing improvements Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 01/17] selftests/bpf: fix RELEASE=1 build for tc_opts Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 02/17] selftests/bpf: satisfy compiler by having explicit return in btf test Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 03/17] bpf: derive smin/smax from umin/max bounds Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 04/17] bpf: derive smin32/smax32 from umin32/umax32 bounds Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 05/17] bpf: derive subreg bounds from full bounds when upper 32 bits are constant Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 06/17] bpf: add special smin32/smax32 derivation from 64-bit bounds Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 07/17] bpf: improve deduction of 64-bit bounds from 32-bit bounds Andrii Nakryiko
2023-11-02 14:39 ` Shung-Hsi Yu
2023-11-02 16:17 ` Andrii Nakryiko
2023-11-03 3:43 ` Shung-Hsi Yu
2023-11-02 3:37 ` [PATCH v6 bpf-next 08/17] bpf: try harder to deduce register bounds from different numeric domains Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 09/17] bpf: drop knowledge-losing __reg_combine_{32,64}_into_{64,32} logic Andrii Nakryiko
2023-11-02 15:14 ` Shung-Hsi Yu
2023-11-02 3:37 ` [PATCH v6 bpf-next 10/17] selftests/bpf: BPF register range bounds tester Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 11/17] bpf: rename is_branch_taken reg arguments to prepare for the second one Andrii Nakryiko
2023-11-02 15:15 ` Shung-Hsi Yu
2023-11-02 3:37 ` [PATCH v6 bpf-next 12/17] bpf: generalize is_branch_taken() to work with two registers Andrii Nakryiko
2023-11-02 15:19 ` Shung-Hsi Yu
2023-11-02 3:37 ` [PATCH v6 bpf-next 13/17] bpf: move is_branch_taken() down Andrii Nakryiko
2023-11-02 3:37 ` Andrii Nakryiko [this message]
2023-11-02 3:37 ` [PATCH v6 bpf-next 15/17] bpf: unify 32-bit and 64-bit is_branch_taken logic Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 16/17] bpf: prepare reg_set_min_max for second set of registers Andrii Nakryiko
2023-11-02 3:37 ` [PATCH v6 bpf-next 17/17] bpf: generalize reg_set_min_max() to handle two sets of two registers Andrii Nakryiko
2023-11-02 16:10 ` [PATCH v6 bpf-next 00/17] BPF register bounds logic and testing improvements patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231102033759.2541186-15-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=kernel-team@meta.com \
--cc=martin.lau@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox