From: Eduard Zingerman <eddyz87@gmail.com>
To: Andrii Nakryiko <andrii@kernel.org>,
bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
martin.lau@kernel.org
Cc: kernel-team@meta.com
Subject: Re: [PATCH v5 bpf-next 14/23] bpf: generalize is_branch_taken to handle all conditional jumps in one place
Date: Tue, 31 Oct 2023 17:38:29 +0200 [thread overview]
Message-ID: <8a9da941cc2aefba27da706e4e9b2d11aedfc86f.camel@gmail.com> (raw)
In-Reply-To: <20231027181346.4019398-15-andrii@kernel.org>
On Fri, 2023-10-27 at 11:13 -0700, Andrii Nakryiko wrote:
> > 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().
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> > ---
> > 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 25b5234ebda3..fedd6d0e76e5 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -14169,6 +14169,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
> > */
> > @@ -14410,12 +14423,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))
> > @@ -14896,27 +14917,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.
next prev parent reply other threads:[~2023-10-31 15:38 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-27 18:13 [PATCH v5 bpf-next 00/23] BPF register bounds logic and testing improvements Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 01/23] selftests/bpf: fix RELEASE=1 build for tc_opts Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 02/23] selftests/bpf: satisfy compiler by having explicit return in btf test Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 03/23] bpf: derive smin/smax from umin/max bounds Andrii Nakryiko
2023-10-31 15:37 ` Eduard Zingerman
2023-10-31 17:30 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 04/23] bpf: derive smin32/smax32 from umin32/umax32 bounds Andrii Nakryiko
2023-10-31 15:37 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 05/23] bpf: derive subreg bounds from full bounds when upper 32 bits are constant Andrii Nakryiko
2023-10-31 15:37 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 06/23] bpf: add special smin32/smax32 derivation from 64-bit bounds Andrii Nakryiko
2023-10-31 15:37 ` Eduard Zingerman
2023-10-31 17:39 ` Andrii Nakryiko
2023-10-31 18:41 ` Alexei Starovoitov
2023-10-31 18:49 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 07/23] bpf: improve deduction of 64-bit bounds from 32-bit bounds Andrii Nakryiko
2023-10-31 15:37 ` Eduard Zingerman
2023-10-31 20:26 ` Alexei Starovoitov
2023-10-31 20:33 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 08/23] bpf: try harder to deduce register bounds from different numeric domains Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 09/23] bpf: drop knowledge-losing __reg_combine_{32,64}_into_{64,32} logic Andrii Nakryiko
2023-10-31 15:38 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 10/23] selftests/bpf: BPF register range bounds tester Andrii Nakryiko
2023-11-08 22:08 ` Eduard Zingerman
2023-11-08 23:23 ` Andrii Nakryiko
2023-11-09 0:30 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 11/23] bpf: rename is_branch_taken reg arguments to prepare for the second one Andrii Nakryiko
2023-10-30 19:39 ` Alexei Starovoitov
2023-10-31 5:19 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 12/23] bpf: generalize is_branch_taken() to work with two registers Andrii Nakryiko
2023-10-31 15:38 ` Eduard Zingerman
2023-10-31 17:41 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 13/23] bpf: move is_branch_taken() down Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 14/23] bpf: generalize is_branch_taken to handle all conditional jumps in one place Andrii Nakryiko
2023-10-31 15:38 ` Eduard Zingerman [this message]
2023-10-27 18:13 ` [PATCH v5 bpf-next 15/23] bpf: unify 32-bit and 64-bit is_branch_taken logic Andrii Nakryiko
2023-10-30 19:52 ` Alexei Starovoitov
2023-10-31 5:28 ` Andrii Nakryiko
2023-10-31 17:35 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 16/23] bpf: prepare reg_set_min_max for second set of registers Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 17/23] bpf: generalize reg_set_min_max() to handle two sets of two registers Andrii Nakryiko
2023-10-31 2:02 ` Alexei Starovoitov
2023-10-31 6:03 ` Andrii Nakryiko
2023-10-31 16:23 ` Alexei Starovoitov
2023-10-31 17:50 ` Andrii Nakryiko
2023-10-31 17:56 ` Andrii Nakryiko
2023-10-31 18:04 ` Alexei Starovoitov
2023-10-31 18:06 ` Andrii Nakryiko
2023-10-31 18:14 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 18/23] bpf: generalize reg_set_min_max() to handle non-const register comparisons Andrii Nakryiko
2023-10-31 23:25 ` Eduard Zingerman
2023-11-01 16:35 ` Andrii Nakryiko
2023-11-01 17:12 ` Eduard Zingerman
2023-10-27 18:13 ` [PATCH v5 bpf-next 19/23] bpf: generalize is_scalar_branch_taken() logic Andrii Nakryiko
2023-10-31 2:12 ` Alexei Starovoitov
2023-10-31 6:12 ` Andrii Nakryiko
2023-10-31 16:34 ` Alexei Starovoitov
2023-10-31 18:01 ` Andrii Nakryiko
2023-10-31 20:53 ` Andrii Nakryiko
2023-10-31 20:55 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 20/23] bpf: enhance BPF_JEQ/BPF_JNE is_branch_taken logic Andrii Nakryiko
2023-10-31 2:20 ` Alexei Starovoitov
2023-10-31 6:16 ` Andrii Nakryiko
2023-10-31 16:36 ` Alexei Starovoitov
2023-10-31 18:04 ` Andrii Nakryiko
2023-10-31 18:06 ` Alexei Starovoitov
2023-10-27 18:13 ` [PATCH v5 bpf-next 21/23] selftests/bpf: adjust OP_EQ/OP_NE handling to use subranges for branch taken Andrii Nakryiko
2023-11-08 18:22 ` Eduard Zingerman
2023-11-08 19:59 ` Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 22/23] selftests/bpf: add range x range test to reg_bounds Andrii Nakryiko
2023-10-27 18:13 ` [PATCH v5 bpf-next 23/23] selftests/bpf: add iter test requiring range x range logic Andrii Nakryiko
2023-10-30 17:55 ` [PATCH v5 bpf-next 00/23] BPF register bounds logic and testing improvements Alexei Starovoitov
2023-10-31 5:19 ` Andrii Nakryiko
2023-11-01 12:37 ` Paul Chaignon
2023-11-01 17:13 ` Andrii Nakryiko
2023-11-07 6:37 ` Harishankar Vishwanathan
2023-11-07 16:38 ` Paul Chaignon
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=8a9da941cc2aefba27da706e4e9b2d11aedfc86f.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.