All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, yonghong.song@linux.dev, memxor@gmail.com,
	awerner32@gmail.com, Eduard Zingerman <eddyz87@gmail.com>
Subject: [PATCH bpf 10/12] bpf: keep track of max number of bpf_loop callback iterations
Date: Thu, 16 Nov 2023 04:18:01 +0200	[thread overview]
Message-ID: <20231116021803.9982-11-eddyz87@gmail.com> (raw)
In-Reply-To: <20231116021803.9982-1-eddyz87@gmail.com>

In some cases verifier can't infer convergence of the bpf_loop()
iteration. E.g. for the following program:

    static int cb(__u32 idx, struct num_context* ctx)
    {
        ctx->i++;
        return 0;
    }

    SEC("?raw_tp")
    int prog(void *_)
    {
        struct num_context ctx = { .i = 0 };
        __u8 choice_arr[2] = { 0, 1 };

        bpf_loop(2, cb, &ctx, 0);
        return choice_arr[ctx.i];
    }

Each 'cb' simulation would eventually return to 'prog' and reach
'return choice_arr[ctx.i]' statement. At which point ctx.i would be
marked precise, thus forcing verifier to track multitude of separate
states with {.i=0}, {.i=1}, ... at bpf_loop() callback entry.

This commit allows "brute force" handling for such cases by limiting
number of callback body simulations using 'umax' value of the first
bpf_loop() parameter.

For this, extend bpf_func_state with 'callback_depth' field.
Increment this field when callback visiting state is pushed to states
traversal stack. For frame #N it's 'callback_depth' field counts how
many times callback with frame depth N+1 had been executed.
Use bpf_func_state specifically to allow independent tracking of
callback depths when multiple nested bpf_loop() calls are present.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 include/linux/bpf_verifier.h |  9 +++++++++
 kernel/bpf/verifier.c        | 12 ++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 0ffb479c72d8..302f9c310de7 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -301,6 +301,15 @@ struct bpf_func_state {
 	struct tnum callback_ret_range;
 	bool in_async_callback_fn;
 	bool in_exception_callback_fn;
+	/* For callback calling functions that limit number of possible
+	 * callback executions (e.g. bpf_loop) keeps track of current
+	 * simulated iteration number. When non-zero either:
+	 * - current frame has a child frame, in such case it's callsite points
+	 *   to callback calling function;
+	 * - current frame is a topmost frame, in such case callback has just
+	 *   returned and env->insn_idx points to callback calling function.
+	 */
+	u32 callback_depth;
 
 	/* The following fields should be last. See copy_func_state() */
 	int acquired_refs;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5b8c0ebcb4f6..474af277ea54 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9680,6 +9680,8 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
 		return err;
 
 	callback_state->callback_iter_depth++;
+	callback_state->frame[callback_state->curframe - 1]->callback_depth++;
+	caller->callback_depth = 0;
 	return 0;
 }
 
@@ -10479,8 +10481,14 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		break;
 	case BPF_FUNC_loop:
 		update_loop_inline_state(env, meta.subprogno);
-		err = push_callback_call(env, insn, insn_idx, meta.subprogno,
-					 set_loop_callback_state);
+		if (env->log.level & BPF_LOG_LEVEL2)
+			verbose(env, "frame%d callback_depth=%u\n",
+				env->cur_state->curframe, cur_func(env)->callback_depth);
+		if (cur_func(env)->callback_depth < regs[BPF_REG_1].umax_value)
+			err = push_callback_call(env, insn, insn_idx, meta.subprogno,
+						 set_loop_callback_state);
+		else
+			cur_func(env)->callback_depth = 0;
 		break;
 	case BPF_FUNC_dynptr_from_mem:
 		if (regs[BPF_REG_1].type != PTR_TO_MAP_VALUE) {
-- 
2.42.0


  parent reply	other threads:[~2023-11-16  2:18 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-16  2:17 [PATCH bpf 00/12] verify callbacks as if they are called unknown number of times Eduard Zingerman
2023-11-16  2:17 ` [PATCH bpf 01/12] selftests/bpf: track tcp payload offset as scalar in xdp_synproxy Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-16  2:17 ` [PATCH bpf 02/12] selftests/bpf: track string payload offset as scalar in strobemeta Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-17 18:52     ` Eduard Zingerman
2023-11-16  2:17 ` [PATCH bpf 03/12] selftests/bpf: fix bpf_loop_bench for new callback verification scheme Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-17 18:52     ` Eduard Zingerman
2023-11-17 21:38   ` Alexei Starovoitov
2023-11-17 21:43     ` Eduard Zingerman
2023-11-17 21:47       ` Alexei Starovoitov
2023-11-17 21:50         ` Eduard Zingerman
2023-11-17 21:55           ` Alexei Starovoitov
2023-11-16  2:17 ` [PATCH bpf 04/12] bpf: extract __check_reg_arg() utility function Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-16  2:17 ` [PATCH bpf 05/12] bpf: extract setup_func_entry() " Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-17 18:52     ` Eduard Zingerman
2023-11-16  2:17 ` [PATCH bpf 06/12] bpf: verify callbacks as if they are called unknown number of times Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-17 18:52     ` Eduard Zingerman
2023-11-17 20:27       ` Andrii Nakryiko
2023-11-17 21:03         ` Eduard Zingerman
2023-11-16  2:17 ` [PATCH bpf 07/12] selftests/bpf: tests for iterating callbacks Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-16  2:17 ` [PATCH bpf 08/12] bpf: widening for callback iterators Eduard Zingerman
2023-11-17 16:46   ` Andrii Nakryiko
2023-11-16  2:18 ` [PATCH bpf 09/12] selftests/bpf: test widening for iterating callbacks Eduard Zingerman
2023-11-17 16:47   ` Andrii Nakryiko
2023-11-17 18:53     ` Eduard Zingerman
2023-11-17 20:28       ` Andrii Nakryiko
2023-11-16  2:18 ` Eduard Zingerman [this message]
2023-11-16 14:08   ` [PATCH bpf 10/12] bpf: keep track of max number of bpf_loop callback iterations Andrii Nakryiko
2023-11-16 14:13     ` Eduard Zingerman
2023-11-17 16:47   ` Andrii Nakryiko
2023-11-17 18:53     ` Eduard Zingerman
2023-11-17 20:30       ` Andrii Nakryiko
2023-11-16  2:18 ` [PATCH bpf 11/12] selftests/bpf: add __not_msg annotation for test_loader based tests Eduard Zingerman
2023-11-17 16:45   ` Andrii Nakryiko
2023-11-17 18:53     ` Eduard Zingerman
2023-11-17 20:31       ` Andrii Nakryiko
2023-11-17 21:10         ` Eduard Zingerman
2023-11-17 21:33           ` Alexei Starovoitov
2023-11-16  2:18 ` [PATCH bpf 12/12] selftests/bpf: check if max number of bpf_loop iterations is tracked Eduard Zingerman
2023-11-17 16:47   ` Andrii Nakryiko
2023-11-17 18:53     ` Eduard Zingerman
2023-11-17 20:32       ` Andrii Nakryiko
2023-11-17 21:18         ` Eduard Zingerman

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=20231116021803.9982-11-eddyz87@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=awerner32@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --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 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.