From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@kernel.org>,
David Vernet <void@manifault.com>, Tejun Heo <tj@kernel.org>,
Raj Sahu <rjsu26@vt.edu>, Dan Williams <djwillia@vt.edu>,
Rishabh Iyer <rishabh.iyer@epfl.ch>,
Sanidhya Kashyap <sanidhya.kashyap@epfl.ch>
Subject: [RFC PATCH v1 01/14] bpf: Mark subprogs as throw reachable before do_check pass
Date: Thu, 1 Feb 2024 04:20:56 +0000 [thread overview]
Message-ID: <20240201042109.1150490-2-memxor@gmail.com> (raw)
In-Reply-To: <20240201042109.1150490-1-memxor@gmail.com>
The motivation of this patch is to figure out which subprogs participate
in exception propagation. In other words, whichever subprog's execution
can lead to an exception being thrown either directly or indirectly (by
way of calling other subprogs).
With the current exceptions support, the runtime performs stack
unwinding when bpf_throw is called. For now, any resources acquired by
the program cannot be released, therefore bpf_throw calls made with
non-zero acquired references must be rejected during verification.
However, there currently exists a loophole in this restriction due to
the way the verification procedure is structured. The verifier will
first walk over the main subprog's instructions, but not descend into
subprog calls to ones with global linkage. These global subprogs will
then be independently verified instead. Therefore, in a situation where
a global subprog ends up throwing an exception (either directly by
calling bpf_throw, or indirectly by way of calling another subprog that
does so), the verifier will fail to notice this fact and may permit
throwing BPF exceptions with non-zero acquired references.
Therefore, to fix this, we add a summarization pass before the do_check
stage which walks all call chains of the program and marks all of the
subprogs that are reachable from a bpf_throw call which unwinds the
program stack.
We only do so if we actually see a bpf_throw call in the program though,
since we do not want to walk all instructions unless we need to. One we
analyze all possible call chains of the program, we will be able to mark
them as 'is_throw_reachable' in their subprog_info.
After performing this step, we need to make another change as to how
subprog call verification occurs. In case of global subprog, we will
need to explore an alternate program path where the call instruction
processing of a global subprog's call will immediately throw an
exception. We will thus simulate a normal path without any exceptions,
and one where the exception is thrown and the program proceeds no
further. In this way, the verifier will be able to detect the whether
any acquired references or locks exist in the verifier state and thus
reject the program if needed.
Fixes: f18b03fabaa9 ("bpf: Implement BPF exceptions")
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
include/linux/bpf_verifier.h | 2 +
kernel/bpf/verifier.c | 86 ++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 0dcde339dc7e..1d666b6c21e6 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -626,6 +626,7 @@ struct bpf_subprog_info {
bool is_async_cb: 1;
bool is_exception_cb: 1;
bool args_cached: 1;
+ bool is_throw_reachable: 1;
u8 arg_cnt;
struct bpf_subprog_arg_info args[MAX_BPF_FUNC_REG_ARGS];
@@ -691,6 +692,7 @@ struct bpf_verifier_env {
bool bypass_spec_v4;
bool seen_direct_write;
bool seen_exception;
+ bool seen_throw_insn;
struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
const struct bpf_line_info *prev_linfo;
struct bpf_verifier_log log;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index cd4d780e5400..bba53c4e3a0c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2941,6 +2941,8 @@ static int check_subprogs(struct bpf_verifier_env *env)
insn[i].src_reg == 0 &&
insn[i].imm == BPF_FUNC_tail_call)
subprog[cur_subprog].has_tail_call = true;
+ if (!env->seen_throw_insn && is_bpf_throw_kfunc(&insn[i]))
+ env->seen_throw_insn = true;
if (BPF_CLASS(code) == BPF_LD &&
(BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
subprog[cur_subprog].has_ld_abs = true;
@@ -5866,6 +5868,9 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx)
if (!is_bpf_throw_kfunc(insn + i))
continue;
+ /* When this is allowed, don't forget to update logic for sync and
+ * async callbacks in mark_exception_reachable_subprogs.
+ */
if (subprog[idx].is_cb)
err = true;
for (int c = 0; c < frame && !err; c++) {
@@ -16205,6 +16210,83 @@ static int check_btf_info(struct bpf_verifier_env *env,
return 0;
}
+/* We walk the call graph of the program in this function, and mark everything in
+ * the call chain as 'is_throw_reachable'. This allows us to know which subprog
+ * calls may propagate an exception and generate exception frame descriptors for
+ * those call instructions. We already do that for bpf_throw calls made directly,
+ * but we need to mark the subprogs as we won't be able to see the call chains
+ * during symbolic execution in do_check_common due to global subprogs.
+ *
+ * Note that unlike check_max_stack_depth, we don't explore the async callbacks
+ * apart from main subprogs, as we don't support throwing from them for now, but
+ */
+static int mark_exception_reachable_subprogs(struct bpf_verifier_env *env)
+{
+ struct bpf_subprog_info *subprog = env->subprog_info;
+ struct bpf_insn *insn = env->prog->insnsi;
+ int idx = 0, frame = 0, i, subprog_end;
+ int ret_insn[MAX_CALL_FRAMES];
+ int ret_prog[MAX_CALL_FRAMES];
+
+ /* No need if we never saw any bpf_throw() call in the program. */
+ if (!env->seen_throw_insn)
+ return 0;
+
+ i = subprog[idx].start;
+restart:
+ subprog_end = subprog[idx + 1].start;
+ for (; i < subprog_end; i++) {
+ int next_insn, sidx;
+
+ if (bpf_pseudo_kfunc_call(insn + i) && !insn[i].off) {
+ if (!is_bpf_throw_kfunc(insn + i))
+ continue;
+ subprog[idx].is_throw_reachable = true;
+ for (int j = 0; j < frame; j++)
+ subprog[ret_prog[j]].is_throw_reachable = true;
+ }
+
+ if (!bpf_pseudo_call(insn + i) && !bpf_pseudo_func(insn + i))
+ continue;
+ /* remember insn and function to return to */
+ ret_insn[frame] = i + 1;
+ ret_prog[frame] = idx;
+
+ /* find the callee */
+ next_insn = i + insn[i].imm + 1;
+ sidx = find_subprog(env, next_insn);
+ if (sidx < 0) {
+ WARN_ONCE(1, "verifier bug. No program starts at insn %d\n", next_insn);
+ return -EFAULT;
+ }
+ /* We cannot distinguish between sync or async cb, so we need to follow
+ * both. Async callbacks don't really propagate exceptions but calling
+ * bpf_throw from them is not allowed anyway, so there is no harm in
+ * exploring them.
+ * TODO: To address this properly, we will have to move is_cb,
+ * is_async_cb markings to the stage before do_check.
+ */
+ i = next_insn;
+ idx = sidx;
+
+ frame++;
+ if (frame >= MAX_CALL_FRAMES) {
+ verbose(env, "the call stack of %d frames is too deep !\n", frame);
+ return -E2BIG;
+ }
+ goto restart;
+ }
+ /* end of for() loop means the last insn of the 'subprog'
+ * was reached. Doesn't matter whether it was JA or EXIT
+ */
+ if (frame == 0)
+ return 0;
+ frame--;
+ i = ret_insn[frame];
+ idx = ret_prog[frame];
+ goto restart;
+}
+
/* check %cur's range satisfies %old's */
static bool range_within(struct bpf_reg_state *old,
struct bpf_reg_state *cur)
@@ -20939,6 +21021,10 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
if (ret < 0)
goto skip_full_check;
+ ret = mark_exception_reachable_subprogs(env);
+ if (ret < 0)
+ goto skip_full_check;
+
ret = do_check_main(env);
ret = ret ?: do_check_subprogs(env);
--
2.40.1
next prev parent reply other threads:[~2024-02-01 4:21 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-01 4:20 [RFC PATCH v1 00/14] Exceptions - Resource Cleanup Kumar Kartikeya Dwivedi
2024-02-01 4:20 ` Kumar Kartikeya Dwivedi [this message]
2024-02-12 19:35 ` [RFC PATCH v1 01/14] bpf: Mark subprogs as throw reachable before do_check pass David Vernet
2024-02-12 22:28 ` Kumar Kartikeya Dwivedi
2024-02-15 1:01 ` Eduard Zingerman
2024-02-16 21:34 ` Kumar Kartikeya Dwivedi
2024-02-01 4:20 ` [RFC PATCH v1 02/14] bpf: Process global subprog's exception propagation Kumar Kartikeya Dwivedi
2024-02-15 1:10 ` Eduard Zingerman
2024-02-16 21:50 ` Kumar Kartikeya Dwivedi
2024-02-17 14:04 ` Eduard Zingerman
2024-02-01 4:20 ` [RFC PATCH v1 03/14] selftests/bpf: Add test for throwing global subprog with acquired refs Kumar Kartikeya Dwivedi
2024-02-15 1:10 ` Eduard Zingerman
2024-02-01 4:20 ` [RFC PATCH v1 04/14] bpf: Refactor check_pseudo_btf_id's BTF reference bump Kumar Kartikeya Dwivedi
2024-02-15 1:11 ` Eduard Zingerman
2024-02-16 21:50 ` Kumar Kartikeya Dwivedi
2024-02-01 4:21 ` [RFC PATCH v1 05/14] bpf: Implement BPF exception frame descriptor generation Kumar Kartikeya Dwivedi
2024-02-15 18:24 ` Eduard Zingerman
2024-02-16 11:23 ` Eduard Zingerman
2024-02-16 22:06 ` Kumar Kartikeya Dwivedi
2024-02-17 17:14 ` Eduard Zingerman
2024-02-20 21:58 ` Kumar Kartikeya Dwivedi
2024-02-16 22:24 ` Kumar Kartikeya Dwivedi
2024-02-01 4:21 ` [RFC PATCH v1 06/14] bpf: Adjust frame descriptor pc on instruction patching Kumar Kartikeya Dwivedi
2024-02-15 16:31 ` Eduard Zingerman
2024-02-16 21:52 ` Kumar Kartikeya Dwivedi
2024-02-17 14:08 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 07/14] bpf: Use hidden subprog trampoline for bpf_throw Kumar Kartikeya Dwivedi
2024-02-15 22:11 ` Eduard Zingerman
2024-02-16 21:59 ` Kumar Kartikeya Dwivedi
2024-02-17 14:22 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 08/14] bpf: Compute used callee saved registers for subprogs Kumar Kartikeya Dwivedi
2024-02-15 22:12 ` Eduard Zingerman
2024-02-16 22:02 ` Kumar Kartikeya Dwivedi
2024-02-17 14:26 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 09/14] bpf, x86: Fix up pc offsets for frame descriptor entries Kumar Kartikeya Dwivedi
2024-02-15 22:12 ` Eduard Zingerman
2024-02-16 13:33 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 10/14] bpf, x86: Implement runtime resource cleanup for exceptions Kumar Kartikeya Dwivedi
2024-02-16 12:02 ` Eduard Zingerman
2024-02-16 22:28 ` Kumar Kartikeya Dwivedi
2024-02-19 12:01 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 11/14] bpf: Release references in verifier state when throwing exceptions Kumar Kartikeya Dwivedi
2024-02-16 12:21 ` Eduard Zingerman
2024-02-01 4:21 ` [RFC PATCH v1 12/14] bpf: Register cleanup dtors for runtime unwinding Kumar Kartikeya Dwivedi
2024-02-01 4:21 ` [RFC PATCH v1 13/14] bpf: Make bpf_throw available to all program types Kumar Kartikeya Dwivedi
2024-02-01 4:21 ` [RFC PATCH v1 14/14] selftests/bpf: Add tests for exceptions runtime cleanup Kumar Kartikeya Dwivedi
2024-02-12 20:53 ` David Vernet
2024-02-12 22:43 ` Kumar Kartikeya Dwivedi
2024-02-13 19:33 ` David Vernet
2024-02-13 20:51 ` Kumar Kartikeya Dwivedi
2024-03-14 11:08 ` [RFC PATCH v1 00/14] Exceptions - Resource Cleanup Eduard Zingerman
2024-03-18 5:40 ` Kumar Kartikeya Dwivedi
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=20240201042109.1150490-2-memxor@gmail.com \
--to=memxor@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=djwillia@vt.edu \
--cc=martin.lau@kernel.org \
--cc=rishabh.iyer@epfl.ch \
--cc=rjsu26@vt.edu \
--cc=sanidhya.kashyap@epfl.ch \
--cc=tj@kernel.org \
--cc=void@manifault.com \
/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