diff mbox series

[RFC,bpf-next,v1,5/9] bpf: Add pass to fixup global function throw information

Message ID 20230405004239.1375399-6-memxor@gmail.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series Exceptions - 1/2 | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-4 pending Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for build for x86_64 with llvm-16
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 28 this patch: 29
netdev/cc_maintainers warning 8 maintainers not CCed: song@kernel.org sdf@google.com haoluo@google.com yhs@fb.com john.fastabend@gmail.com kpsingh@kernel.org jolsa@kernel.org martin.lau@linux.dev
netdev/build_clang fail Errors and warnings before: 18 this patch: 20
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 28 this patch: 29
netdev/checkpatch warning WARNING: line length of 116 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 96 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kumar Kartikeya Dwivedi April 5, 2023, 12:42 a.m. UTC
Within the do_check pass, we made a core assumption that we have correct
can_throw info about all global subprogs and simply used
mark_chain_throw without entering them to mark callsites leading up to
their call. However, the do_check_subprogs pass of verifier is iterative
and does not propagate can_throw information across global subprogs
which call into each other. We need an extra pass through all of them to
propagate can_throw information visibility throwing global subprogs into
global subprogs that call into them.

After doing this pass, do_check_main will directly use mark_chain_throw
again and have the correct information about all global subprogs which
are called by it.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 118 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 117 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 07d808b05044..acfcaadca3b6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13664,6 +13664,12 @@  static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
 			verbose(env, "missing btf func_info\n");
 			return -EINVAL;
 		}
+		/* NOTE: Do not change this directly, as we rely on only
+		 * BPF_FUNC_STATIC allowed as BPF_PSEUDO_FUNC targets in
+		 * do_check_subprogs, see comment about propagating exception
+		 * information across global functions. When changing this, add
+		 * bpf_pseudo_func handling to the propagating loop as well.
+		 */
 		if (aux->func_info_aux[subprogno].linkage != BTF_FUNC_STATIC) {
 			verbose(env, "callback function not static\n");
 			return -EINVAL;
@@ -18491,6 +18497,110 @@  static int do_check_common(struct bpf_verifier_env *env, int subprog)
 	return ret;
 }
 
+/* We have gone through all global subprogs, and we know which ones were seen as
+ * throwing exceptions. Since calls to other global functions are not explored
+ * and we simply continue exploration at the next instruction, we may have not
+ * fully propagated can_throw information. E.g. consider the case below, where 1
+ * and 2 are verified in order.
+ *
+ * gfunc 1:
+ *	call gfunc2
+ *	exit
+ * gfunc 2:
+ *	call bpf_throw
+ *
+ * At this point, gfunc1 is not marked as throwing, but it calls gfunc2 which
+ * actually throws. The only thing we need to do is go through every global
+ * function, and propagate the information back to their callers. We only care
+ * about BPF_PSEUDO_CALL, as BPF_PSEUDO_FUNC loads cannot have global functions
+ * as targets
+ *
+ * Logic mostly mimics check_max_stack_depth, but adjusted and simplified for
+ * our use case.
+ */
+static int fixup_global_subprog_throw_info(struct bpf_verifier_env *env)
+{
+	struct bpf_func_info_aux *func_info_aux = env->prog->aux->func_info_aux;
+	struct bpf_subprog_info *subprog = env->subprog_info;
+	int frame = 0, idx = 0, i = 0, subprog_end;
+	struct bpf_insn *insn = env->prog->insnsi;
+	int ret_insn[MAX_CALL_FRAMES];
+	int ret_prog[MAX_CALL_FRAMES];
+	bool can_throw;
+	int j, ret;
+
+	/* Start at first global subprog */
+	for (int s = 1; s < env->subprog_cnt; s++) {
+		if (func_info_aux[s].linkage != BTF_FUNC_GLOBAL)
+			continue;
+		idx = s;
+		break;
+	}
+	if (!idx)
+		return -EFAULT;
+	i = subprog[idx].start;
+continue_func:
+	can_throw = false;
+	subprog_end = subprog[idx + 1].start;
+	for (; i < subprog_end; i++) {
+		int next_insn;
+
+		if (!bpf_pseudo_call(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;
+		idx = find_subprog(env, next_insn);
+		if (idx < 0) {
+			WARN_ONCE(1, "verifier bug. No program starts at insn %d\n", next_insn);
+			return -EFAULT;
+		}
+
+		/* Only follow global subprog calls */
+		if (func_info_aux[idx].linkage != BTF_FUNC_GLOBAL)
+			continue;
+		/* If this subprog already throws, mark all callers and continue
+		 * with next instruction in current subprog.
+		 */
+		if (subprog[idx].can_throw) {
+			/* Include current frame info when marking */
+			for (j = frame; j >= 0; j--) {
+				func_info_aux[ret_prog[j]].throws_exception = subprog[ret_prog[j]].can_throw = true;
+				/* Exception subprog cannot be set in global
+				 * function context, so set_throw_state_type
+				 * will always mark type as BPF_THROW_INNER
+				 * and subprog as -1.
+				 */
+				ret = set_throw_state_type(env, ret_insn[j] - 1, j, ret_prog[j]);
+				if (ret < 0)
+					return ret;
+			}
+			continue;
+		}
+
+		i = next_insn;
+		frame++;
+		if (frame >= MAX_CALL_FRAMES) {
+			verbose(env, "the call stack of %d frames is too deep !\n",
+				frame);
+			return -E2BIG;
+		}
+		goto continue_func;
+	}
+	/* 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 continue_func;
+}
+
 /* Verify all global functions in a BPF program one by one based on their BTF.
  * All global functions must pass verification. Otherwise the whole program is rejected.
  * Consider:
@@ -18511,6 +18621,7 @@  static int do_check_common(struct bpf_verifier_env *env, int subprog)
 static int do_check_subprogs(struct bpf_verifier_env *env)
 {
 	struct bpf_prog_aux *aux = env->prog->aux;
+	bool does_anyone_throw = false;
 	int i, ret;
 
 	if (!aux->func_info)
@@ -18535,8 +18646,13 @@  static int do_check_subprogs(struct bpf_verifier_env *env)
 		 * opposite is fine though.
 		 */
 		aux->func_info_aux[i].throws_exception = env->subprog_info[i].can_throw;
+		if (!does_anyone_throw && env->subprog_info[i].can_throw)
+			does_anyone_throw = true;
 	}
-	return 0;
+
+	if (!does_anyone_throw)
+		return 0;
+	return fixup_global_subprog_throw_info(env);
 }
 
 static int do_check_main(struct bpf_verifier_env *env)