netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <ast@kernel.org>
To: "David S . Miller" <davem@davemloft.net>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
	Jann Horn <jannh@google.com>, <netdev@vger.kernel.org>,
	<kernel-team@fb.com>
Subject: [PATCH bpf-next 1/2] bpf: fix maximum stack depth tracking logic
Date: Fri, 22 Dec 2017 13:33:27 -0800	[thread overview]
Message-ID: <20171222213328.993019-2-ast@kernel.org> (raw)
In-Reply-To: <20171222213328.993019-1-ast@kernel.org>

instead of computing max stack depth for current call chain only
track the maximum possible stack depth of any function at given
frame position. Such algorithm is simple and fast, but conservative,
since it overestimates amount of stack used. Consider:
main() // stack 32
{
  A();
  B();
}

A(){} // stack 256

B()  // stack 64
{
  A();
}

since A() is called at frame[1] and frame[2], the algorithm
will estimate the max stack depth as 32 + 256 + 256 and will reject
such program, though real max is 32 + 64 + 256.

Fortunately the algorithm is good enough in practice. The alternative
would be to track max stack of every function in the fast pass through
the verifier and then do additional CFG walk just to compute max total.

Fixes: f4d7e40a5b71 ("bpf: introduce function calls (verification)")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/bpf_verifier.h | 17 +++++++++++++++++
 kernel/bpf/verifier.c        | 15 +++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index aaac589e490c..adc65ef093d1 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -194,7 +194,24 @@ struct bpf_verifier_env {
 	struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
 	struct bpf_verifer_log log;
 	u32 subprog_starts[BPF_MAX_SUBPROGS];
+	/* computes the stack depth of each bpf function */
 	u16 subprog_stack_depth[BPF_MAX_SUBPROGS + 1];
+	/* gradually computes the maximum stack depth out of all functions
+	 * called at this frame position. Example:
+	 * main()
+	 * {
+	 *   a();
+	 *   b();
+	 * }
+	 * b()
+	 * {
+	 *   a();
+	 * }
+	 * frame_stack_depth[0] = stack depth of main()
+	 * frame_stack_depth[1] = max { stack depth of a(), stack depth of b() }
+	 * frame_stack_depth[2] = stack depth of a()
+	 */
+	u16 frame_stack_depth[MAX_CALL_FRAMES];
 	u32 subprog_cnt;
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4ae46b2cba88..096681bcb312 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1434,15 +1434,18 @@ static int update_stack_depth(struct bpf_verifier_env *env,
 	struct bpf_verifier_state *cur = env->cur_state;
 	int i;
 
+	if (stack < -off)
+		/* update known max for given subprogram */
+		env->subprog_stack_depth[func->subprogno] = -off;
+
+	stack = env->frame_stack_depth[func->frameno];
 	if (stack >= -off)
 		return 0;
+	env->frame_stack_depth[func->frameno] = -off;
 
-	/* update known max for given subprogram */
-	env->subprog_stack_depth[func->subprogno] = -off;
-
-	/* compute the total for current call chain */
-	for (i = 0; i <= cur->curframe; i++) {
-		u32 depth = env->subprog_stack_depth[cur->frame[i]->subprogno];
+	/* some frame changed its max, recompute the total */
+	for (i = 0; i < MAX_CALL_FRAMES; i++) {
+		u32 depth = env->frame_stack_depth[i];
 
 		/* round up to 32-bytes, since this is granularity
 		 * of interpreter stack sizes
-- 
2.9.5

  reply	other threads:[~2017-12-22 21:33 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-22 21:33 [PATCH bpf-next 0/2] bpf: stack depth tracking fix Alexei Starovoitov
2017-12-22 21:33 ` Alexei Starovoitov [this message]
2017-12-23  1:38   ` [PATCH bpf-next 1/2] bpf: fix maximum stack depth tracking logic Jann Horn
2017-12-23  2:07     ` Alexei Starovoitov
2017-12-23  2:26       ` Jann Horn
2017-12-23  3:03         ` Alexei Starovoitov
2017-12-22 21:33 ` [PATCH bpf-next 2/2] selftests/bpf: additional stack depth tests 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=20171222213328.993019-2-ast@kernel.org \
    --to=ast@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jannh@google.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.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;
as well as URLs for NNTP newsgroup(s).