bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maxim Mikityanskiy <maxtram95@gmail.com>
To: Eduard Zingerman <eddyz87@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Shung-Hsi Yu <shung-hsi.yu@suse.com>
Cc: John Fastabend <john.fastabend@gmail.com>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Mykola Lysenko <mykolal@fb.com>,
	Shuah Khan <shuah@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
	netdev@vger.kernel.org, Maxim Mikityanskiy <maxim@isovalent.com>
Subject: [PATCH bpf-next v2 10/15] bpf: Track spilled unbounded scalars
Date: Mon,  8 Jan 2024 22:52:04 +0200	[thread overview]
Message-ID: <20240108205209.838365-11-maxtram95@gmail.com> (raw)
In-Reply-To: <20240108205209.838365-1-maxtram95@gmail.com>

From: Maxim Mikityanskiy <maxim@isovalent.com>

Support the pattern where an unbounded scalar is spilled to the stack,
then boundary checks are performed on the src register, after which the
stack frame slot is refilled into a register.

Before this commit, the verifier didn't treat the src register and the
stack slot as related if the src register was an unbounded scalar. The
register state wasn't copied, the id wasn't preserved, and the stack
slot was marked as STACK_MISC. Subsequent boundary checks on the src
register wouldn't result in updating the boundaries of the spilled
variable on the stack.

After this commit, the verifier will preserve the bond between src and
dst even if src is unbounded, which permits to do boundary checks on src
and refill dst later, still remembering its boundaries. Such a pattern
is sometimes generated by clang when compiling complex long functions.

One test is adjusted to reflect the fact that an untracked register is
marked as precise at an earlier stage, and one more test is adjusted to
reflect that now unbounded scalars are tracked.

Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/verifier.c                                   | 7 +------
 tools/testing/selftests/bpf/progs/verifier_spill_fill.c | 6 +++---
 tools/testing/selftests/bpf/verifier/precise.c          | 6 +++---
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 055fa8096a08..e7fff5f5aa1d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4389,11 +4389,6 @@ static bool __is_scalar_unbounded(struct bpf_reg_state *reg)
 	       reg->u32_min_value == 0 && reg->u32_max_value == U32_MAX;
 }
 
-static bool register_is_bounded(struct bpf_reg_state *reg)
-{
-	return reg->type == SCALAR_VALUE && !__is_scalar_unbounded(reg);
-}
-
 static bool __is_pointer_value(bool allow_ptr_leaks,
 			       const struct bpf_reg_state *reg)
 {
@@ -4504,7 +4499,7 @@ static int check_stack_write_fixed_off(struct bpf_verifier_env *env,
 		return err;
 
 	mark_stack_slot_scratched(env, spi);
-	if (reg && !(off % BPF_REG_SIZE) && register_is_bounded(reg) && env->bpf_capable) {
+	if (reg && !(off % BPF_REG_SIZE) && reg->type == SCALAR_VALUE && env->bpf_capable) {
 		bool reg_value_fits;
 
 		reg_value_fits = get_reg_width(reg) <= BITS_PER_BYTE * size;
diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
index b05aab925ee5..57eb70e100a3 100644
--- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
+++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
@@ -452,9 +452,9 @@ l0_%=:	r1 >>= 16;					\
 SEC("raw_tp")
 __log_level(2)
 __success
-__msg("fp-8=0m??mmmm")
-__msg("fp-16=00mm??mm")
-__msg("fp-24=00mm???m")
+__msg("fp-8=0m??scalar()")
+__msg("fp-16=00mm??scalar()")
+__msg("fp-24=00mm???scalar()")
 __naked void spill_subregs_preserve_stack_zero(void)
 {
 	asm volatile (
diff --git a/tools/testing/selftests/bpf/verifier/precise.c b/tools/testing/selftests/bpf/verifier/precise.c
index 8a2ff81d8350..0a9293a57211 100644
--- a/tools/testing/selftests/bpf/verifier/precise.c
+++ b/tools/testing/selftests/bpf/verifier/precise.c
@@ -183,10 +183,10 @@
 	.prog_type = BPF_PROG_TYPE_XDP,
 	.flags = BPF_F_TEST_STATE_FREQ,
 	.errstr = "mark_precise: frame0: last_idx 7 first_idx 7\
-	mark_precise: frame0: parent state regs=r4 stack=:\
+	mark_precise: frame0: parent state regs=r4 stack=-8:\
 	mark_precise: frame0: last_idx 6 first_idx 4\
-	mark_precise: frame0: regs=r4 stack= before 6: (b7) r0 = -1\
-	mark_precise: frame0: regs=r4 stack= before 5: (79) r4 = *(u64 *)(r10 -8)\
+	mark_precise: frame0: regs=r4 stack=-8 before 6: (b7) r0 = -1\
+	mark_precise: frame0: regs=r4 stack=-8 before 5: (79) r4 = *(u64 *)(r10 -8)\
 	mark_precise: frame0: regs= stack=-8 before 4: (7b) *(u64 *)(r3 -8) = r0\
 	mark_precise: frame0: parent state regs=r0 stack=:\
 	mark_precise: frame0: last_idx 3 first_idx 3\
-- 
2.43.0


  parent reply	other threads:[~2024-01-08 20:53 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-08 20:51 [PATCH bpf-next v2 00/15] Improvements for tracking scalars in the BPF verifier Maxim Mikityanskiy
2024-01-08 20:51 ` [PATCH bpf-next v2 01/15] selftests/bpf: Fix the u64_offset_to_skb_data test Maxim Mikityanskiy
2024-01-08 20:51 ` [PATCH bpf-next v2 02/15] bpf: make infinite loop detection in is_state_visited() exact Maxim Mikityanskiy
2024-01-08 20:51 ` [PATCH bpf-next v2 03/15] selftests/bpf: check if imprecise stack spills confuse infinite loop detection Maxim Mikityanskiy
2024-01-08 20:51 ` [PATCH bpf-next v2 04/15] bpf: Make bpf_for_each_spilled_reg consider narrow spills Maxim Mikityanskiy
2024-01-08 20:51 ` [PATCH bpf-next v2 05/15] selftests/bpf: Add a test case for 32-bit spill tracking Maxim Mikityanskiy
2024-01-08 20:52 ` [PATCH bpf-next v2 06/15] bpf: Add the assign_scalar_id_before_mov function Maxim Mikityanskiy
2024-01-08 20:52 ` [PATCH bpf-next v2 07/15] bpf: Add the get_reg_width function Maxim Mikityanskiy
2024-01-08 20:52 ` [PATCH bpf-next v2 08/15] bpf: Assign ID to scalars on spill Maxim Mikityanskiy
2024-01-08 20:52 ` [PATCH bpf-next v2 09/15] selftests/bpf: Test assigning " Maxim Mikityanskiy
2024-01-09 23:34   ` Andrii Nakryiko
2024-01-08 20:52 ` Maxim Mikityanskiy [this message]
2024-01-12 19:10   ` [PATCH bpf-next v2 10/15] bpf: Track spilled unbounded scalars Alexei Starovoitov
2024-01-12 20:44     ` Maxim Mikityanskiy
2024-01-12 20:50       ` Alexei Starovoitov
2024-01-08 20:52 ` [PATCH bpf-next v2 11/15] selftests/bpf: Test tracking " Maxim Mikityanskiy
2024-01-08 20:52 ` [PATCH bpf-next v2 12/15] bpf: Preserve boundaries and track scalars on narrowing fill Maxim Mikityanskiy
2024-01-09 23:51   ` Andrii Nakryiko
2024-01-08 20:52 ` [PATCH bpf-next v2 13/15] selftests/bpf: Add test cases for " Maxim Mikityanskiy
2024-01-09 23:55   ` Andrii Nakryiko
2024-01-08 20:52 ` [PATCH bpf-next v2 14/15] bpf: Optimize state pruning for spilled scalars Maxim Mikityanskiy
2024-01-10  0:22   ` Andrii Nakryiko
2024-01-10 21:04     ` Eduard Zingerman
2024-01-10 21:52       ` Andrii Nakryiko
2024-01-08 20:52 ` [PATCH bpf-next v2 15/15] selftests/bpf: states pruning checks for scalar vs STACK_{MISC,ZERO} Maxim Mikityanskiy
2024-01-10  0:27   ` Andrii Nakryiko
2024-01-10 20:27     ` Eduard Zingerman
2024-01-12  3:00 ` [PATCH bpf-next v2 00/15] Improvements for tracking scalars in the BPF verifier 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=20240108205209.838365-11-maxtram95@gmail.com \
    --to=maxtram95@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=maxim@isovalent.com \
    --cc=mykolal@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.com \
    --cc=shuah@kernel.org \
    --cc=shung-hsi.yu@suse.com \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /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;
as well as URLs for NNTP newsgroup(s).