From: Yonghong Song <yhs@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>, <kernel-team@fb.com>,
Martin KaFai Lau <martin.lau@kernel.org>
Subject: [PATCH bpf-next 1/7] bpf: Improve verifier JEQ/JNE insn branch taken checking
Date: Wed, 29 Mar 2023 22:56:05 -0700 [thread overview]
Message-ID: <20230330055605.88807-1-yhs@fb.com> (raw)
In-Reply-To: <20230330055600.86870-1-yhs@fb.com>
Currently, for BPF_JEQ/BPF_JNE insn, verifier determines
whether the branch is taken or not only if both operands
are constants. Therefore, for the following code snippet,
0: (85) call bpf_ktime_get_ns#5 ; R0_w=scalar()
1: (a5) if r0 < 0x3 goto pc+2 ; R0_w=scalar(umin=3)
2: (b7) r2 = 2 ; R2_w=2
3: (1d) if r0 == r2 goto pc+2 6
At insn 3, since r0 is not a constant, verifier assumes both branch
can be taken which may lead inproper verification failure.
Add comparing umin value and the constant. If the umin value
is greater than the constant, for JEQ the branch must be
not-taken, and for JNE the branch must be taken.
The jmp32 mode JEQ/JNE branch taken checking is also
handled similarly.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/verifier.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 20eb2015842f..90bb6d25bc9c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12597,10 +12597,14 @@ static int is_branch32_taken(struct bpf_reg_state *reg, u32 val, u8 opcode)
case BPF_JEQ:
if (tnum_is_const(subreg))
return !!tnum_equals_const(subreg, val);
+ else if (reg->u32_min_value > val)
+ return 0;
break;
case BPF_JNE:
if (tnum_is_const(subreg))
return !tnum_equals_const(subreg, val);
+ else if (reg->u32_min_value > val)
+ return 1;
break;
case BPF_JSET:
if ((~subreg.mask & subreg.value) & val)
@@ -12670,10 +12674,14 @@ static int is_branch64_taken(struct bpf_reg_state *reg, u64 val, u8 opcode)
case BPF_JEQ:
if (tnum_is_const(reg->var_off))
return !!tnum_equals_const(reg->var_off, val);
+ else if (reg->umin_value > val)
+ return 0;
break;
case BPF_JNE:
if (tnum_is_const(reg->var_off))
return !tnum_equals_const(reg->var_off, val);
+ else if (reg->umin_value > val)
+ return 1;
break;
case BPF_JSET:
if ((~reg->var_off.mask & reg->var_off.value) & val)
--
2.34.1
next prev parent reply other threads:[~2023-03-30 5:56 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-30 5:56 [PATCH bpf-next 0/7] bpf: Improve verifier for cond_op and spilled loop index variables Yonghong Song
2023-03-30 5:56 ` Yonghong Song [this message]
2023-03-30 21:14 ` [PATCH bpf-next 1/7] bpf: Improve verifier JEQ/JNE insn branch taken checking Dave Marchevsky
2023-03-31 6:40 ` Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 2/7] selftests/bpf: Add tests for non-constant cond_op NE/EQ bound deduction Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 3/7] bpf: Improve handling of pattern '<const> <cond_op> <non_const>' in verifier Yonghong Song
2023-03-30 22:54 ` Dave Marchevsky
2023-03-31 15:26 ` Yonghong Song
2023-04-04 22:04 ` Andrii Nakryiko
2023-04-06 16:51 ` Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 4/7] selftests/bpf: Add verifier tests for code pattern '<const> <cond_op> <non_const>' Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 5/7] bpf: Mark potential spilled loop index variable as precise Yonghong Song
2023-03-31 21:54 ` Eduard Zingerman
2023-03-31 23:39 ` Yonghong Song
2023-04-03 1:48 ` Alexei Starovoitov
2023-04-03 4:04 ` Yonghong Song
2023-04-04 22:09 ` Andrii Nakryiko
2023-04-06 16:55 ` Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 6/7] selftests/bpf: Remove previous workaround for test verif_scale_loop6 Yonghong Song
2023-03-30 5:56 ` [PATCH bpf-next 7/7] selftests/bpf: Add a new test based on loop6.c Yonghong Song
2023-04-03 1:39 ` Alexei Starovoitov
2023-04-03 3:59 ` Yonghong Song
2023-04-04 21:46 ` [PATCH bpf-next 0/7] bpf: Improve verifier for cond_op and spilled loop index variables Andrii Nakryiko
2023-04-06 16:49 ` Yonghong Song
2023-04-06 18:38 ` Andrii Nakryiko
2023-04-06 19:54 ` Alexei Starovoitov
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=20230330055605.88807-1-yhs@fb.com \
--to=yhs@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.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