public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
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>,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH bpf-next v7 3/9] bpf: Check potential private stack recursion for progs with async callback
Date: Tue, 29 Oct 2024 15:16:52 -0700	[thread overview]
Message-ID: <20241029221652.265284-1-yonghong.song@linux.dev> (raw)
In-Reply-To: <20241029221637.264348-1-yonghong.song@linux.dev>

In previous patch, tracing progs are enabled for private stack since
recursion checking ensures there exists no nested same bpf prog run on
the same cpu.

But it is still possible for nested bpf subprog run on the same cpu
if the same subprog is called in both main prog and async callback,
or in different async callbacks. For example,
  main_prog
   bpf_timer_set_callback(timer, timer_cb);
   call sub1
  sub1
   ...
  time_cb
   call sub1

In the above case, nested subprog run for sub1 is possible with one in
process context and the other in softirq context. If this is the case,
the verifier will disable private stack for this bpf prog.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 kernel/bpf/verifier.c | 46 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d3f4cbab97bc..596afd29f088 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6070,7 +6070,8 @@ static int round_up_stack_depth(struct bpf_verifier_env *env, int stack_depth)
  */
 static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
 					 int *subtree_depth, int *depth_frame,
-					 int priv_stack_supported)
+					 int priv_stack_supported,
+					 char *subprog_visited)
 {
 	struct bpf_subprog_info *subprog = env->subprog_info;
 	struct bpf_insn *insn = env->prog->insnsi;
@@ -6120,8 +6121,12 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
 					idx, subprog_depth);
 				return -EACCES;
 			}
-			if (subprog_depth >= BPF_PRIV_STACK_MIN_SIZE)
+			if (subprog_depth >= BPF_PRIV_STACK_MIN_SIZE) {
 				subprog[idx].use_priv_stack = true;
+				subprog_visited[idx] = 1;
+			}
+		} else {
+			subprog_visited[idx] = 1;
 		}
 	}
 continue_func:
@@ -6222,19 +6227,42 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
 static int check_max_stack_depth(struct bpf_verifier_env *env)
 {
 	struct bpf_subprog_info *si = env->subprog_info;
+	char *subprogs1 = NULL, *subprogs2 = NULL;
 	int ret, subtree_depth = 0, depth_frame;
+	int orig_priv_stack_supported;
 	int priv_stack_supported;
 
 	priv_stack_supported = bpf_enable_priv_stack(env);
 	if (priv_stack_supported < 0)
 		return priv_stack_supported;
 
+	orig_priv_stack_supported = priv_stack_supported;
+	if (orig_priv_stack_supported != NO_PRIV_STACK) {
+		subprogs1 = kvmalloc(env->subprog_cnt * 2, __GFP_ZERO);
+		if (!subprogs1)
+			priv_stack_supported = NO_PRIV_STACK;
+		else
+			subprogs2 = subprogs1 + env->subprog_cnt;
+	}
+
 	for (int i = 0; i < env->subprog_cnt; i++) {
 		if (!i || si[i].is_async_cb) {
 			ret = check_max_stack_depth_subprog(env, i, &subtree_depth, &depth_frame,
-							    priv_stack_supported);
+							    priv_stack_supported, subprogs2);
 			if (ret < 0)
-				return ret;
+				goto out;
+
+			if (priv_stack_supported != NO_PRIV_STACK) {
+				for (int j = 0; j < env->subprog_cnt; j++) {
+					if (subprogs1[j] && subprogs2[j]) {
+						priv_stack_supported = NO_PRIV_STACK;
+						break;
+					}
+					subprogs1[j] |= subprogs2[j];
+				}
+			}
+			if (priv_stack_supported != NO_PRIV_STACK)
+				memset(subprogs2, 0, env->subprog_cnt);
 		}
 	}
 	if (priv_stack_supported == NO_PRIV_STACK) {
@@ -6243,10 +6271,18 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
 				depth_frame, subtree_depth);
 			return -EACCES;
 		}
+		if (orig_priv_stack_supported == PRIV_STACK_ADAPTIVE) {
+			for (int i = 0; i < env->subprog_cnt; i++)
+				si[i].use_priv_stack = false;
+		}
 	}
 	if (si[0].use_priv_stack)
 		env->prog->aux->use_priv_stack = true;
-	return 0;
+	ret = 0;
+
+out:
+	kvfree(subprogs1);
+	return ret;
 }
 
 #ifndef CONFIG_BPF_JIT_ALWAYS_ON
-- 
2.43.5


  parent reply	other threads:[~2024-10-29 22:16 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-29 22:16 [PATCH bpf-next v7 0/9] bpf: Support private stack for bpf progs Yonghong Song
2024-10-29 22:16 ` [PATCH bpf-next v7 1/9] bpf: Check stack depth limit after visiting all subprogs Yonghong Song
2024-10-29 22:16 ` [PATCH bpf-next v7 2/9] bpf: Allow private stack to have each subprog having stack size of 512 bytes Yonghong Song
2024-10-29 22:16 ` Yonghong Song [this message]
2024-10-29 22:16 ` [PATCH bpf-next v7 4/9] bpf: Allocate private stack for eligible main prog or subprogs Yonghong Song
2024-10-29 22:17 ` [PATCH bpf-next v7 5/9] bpf, x86: Avoid repeated usage of bpf_prog->aux->stack_depth Yonghong Song
2024-10-29 22:17 ` [PATCH bpf-next v7 6/9] bpf, x86: Support private stack in jit Yonghong Song
2024-10-29 22:17 ` [PATCH bpf-next v7 7/9] selftests/bpf: Add tracing prog private stack tests Yonghong Song
2024-10-29 22:17 ` [PATCH bpf-next v7 8/9] bpf: Support private stack for struct_ops progs Yonghong Song
2024-10-30 23:14   ` Tejun Heo
2024-10-29 22:17 ` [PATCH bpf-next v7 9/9] selftests/bpf: Add struct_ops prog private stack tests Yonghong Song
2024-10-30 23:29   ` Tejun Heo
2024-10-31 19:37     ` Yonghong Song

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=20241029221652.265284-1-yonghong.song@linux.dev \
    --to=yonghong.song@linux.dev \
    --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 \
    --cc=tj@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