[RFC,v1,09/14] bpf, x86: Fix up pc offsets for frame descriptor entries

Commit Message

Kumar Kartikeya Dwivedi Feb. 1, 2024, 4:21 a.m. UTC

Until now, the program counter value stored in frame descriptor entries
was the instruction index of the BPF program's insn and callsites when
going down the frames in a call chain. However, at runtime, the program
counter will be the pointer to the next instruction, and thus needs to
be computed in a position independent way to tally it at runtime to find
the frame descriptor when unwinding.

To do this, we first convert the global instruction index into an
instruction index relative to the start of a subprog, and add 1 to it
(to reflect that at runtime, the program counter points to the next
instruction). Then, we modify the JIT (for now, x86) to convert them
to instruction offsets relative to the start of the JIT image, which is
the prog->bpf_func of the subprog in question at runtime.

Later, subtracting the prog->bpf_func pointer from runtime program
counter will yield the same offset, and allow us to figure out the
corresponding frame descriptor entry.

Note that we have to mark a frame descriptor entry as 'final' because
bpf_int_jit_compile can be called multiple times, and we would try to
convert our already converted pc values again, therefore once we do the
conversion remember it and do not repeat it.

Signed-off-by: Kumar Kartikeya Dwivedi <[e-mail]>
---
 arch/x86/net/bpf_jit_comp.c | 11 +++++++++++
 include/linux/bpf.h         |  2 ++
 kernel/bpf/verifier.c       | 15 +++++++++++++++
 3 files changed, 28 insertions(+)

Comments

Eduard Zingerman Feb. 15, 2024, 10:12 p.m. UTC | #1

On Thu, 2024-02-01 at 04:21 +0000, Kumar Kartikeya Dwivedi wrote:
> Until now, the program counter value stored in frame descriptor entries
> was the instruction index of the BPF program's insn and callsites when
> going down the frames in a call chain. However, at runtime, the program
> counter will be the pointer to the next instruction, and thus needs to
> be computed in a position independent way to tally it at runtime to find
> the frame descriptor when unwinding.
> 
> To do this, we first convert the global instruction index into an
> instruction index relative to the start of a subprog, and add 1 to it
> (to reflect that at runtime, the program counter points to the next
> instruction). Then, we modify the JIT (for now, x86) to convert them
> to instruction offsets relative to the start of the JIT image, which is
> the prog->bpf_func of the subprog in question at runtime.
> 
> Later, subtracting the prog->bpf_func pointer from runtime program
> counter will yield the same offset, and allow us to figure out the
> corresponding frame descriptor entry.

Would it be possible to instead embed an address (or index)
of the corresponding frame descriptor in instruction stream itself?
E.g. do LD_imm64 and pass it as a first parameter for bpf_throw()?
Thus avoiding the necessity to track ip changes.

[...]

Eduard Zingerman Feb. 16, 2024, 1:33 p.m. UTC | #2

On Fri, 2024-02-16 at 00:12 +0200, Eduard Zingerman wrote:
[...]

> Would it be possible to instead embed an address (or index)
> of the corresponding frame descriptor in instruction stream itself?
> E.g. do LD_imm64 and pass it as a first parameter for bpf_throw()?
> Thus avoiding the necessity to track ip changes.

This won't work for calls upper in the chain.
Oh, well...

Patch

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 87692d983ffd..0dd0791c6ee0 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -3112,6 +3112,17 @@  struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		prog = orig_prog;
 	}
 
+	if (prog->aux->fdtab && !prog->aux->fdtab->final && image) {
+		struct bpf_exception_frame_desc_tab *fdtab = prog->aux->fdtab;
+
+		for (int i = 0; i < fdtab->cnt; i++) {
+			struct bpf_exception_frame_desc *desc = fdtab->desc[i];
+
+			desc->pc = addrs[desc->pc];
+		}
+		prog->aux->fdtab->final = true;
+	}
+
 	if (!image || !prog->is_func || extra_pass) {
 		if (image)
 			bpf_prog_fill_jited_linfo(prog, addrs + 1);
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4ac6add0cec8..e310d3ceb14e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1460,6 +1460,7 @@  struct bpf_prog_aux {
 	bool xdp_has_frags;
 	bool exception_cb;
 	bool exception_boundary;
+	bool bpf_throw_tramp;
 	bool callee_regs_used[4];
 	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
 	const struct btf_type *attach_func_proto;
@@ -3395,6 +3396,7 @@  struct bpf_exception_frame_desc {
 
 struct bpf_exception_frame_desc_tab {
 	u32 cnt;
+	bool final;
 	struct bpf_exception_frame_desc **desc;
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index aeaf97b0a749..ec9acadc9ea8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19514,6 +19514,20 @@  static int jit_subprogs(struct bpf_verifier_env *env)
 		func[i]->aux->exception_cb = env->subprog_info[i].is_exception_cb;
 		if (!i)
 			func[i]->aux->exception_boundary = env->seen_exception;
+		if (i == env->bpf_throw_tramp_subprog)
+			func[i]->aux->bpf_throw_tramp = true;
+		/* Fix up pc of fdtab entries to be relative to subprog start before JIT. */
+		if (env->subprog_info[i].fdtab) {
+			for (int k = 0; k < env->subprog_info[i].fdtab->cnt; k++) {
+				struct bpf_exception_frame_desc *desc = env->subprog_info[i].fdtab->desc[k];
+				/* Add 1 to point to the next instruction, which will be the PC at runtime. */
+				desc->pc = desc->pc - subprog_start + 1;
+			}
+		}
+		/* Transfer fdtab to subprog->aux */
+		func[i]->aux->fdtab = env->subprog_info[i].fdtab;
+		env->subprog_info[i].fdtab = NULL;
+
 		func[i] = bpf_int_jit_compile(func[i]);
 		if (!func[i]->jited) {
 			err = -ENOTSUPP;
@@ -19604,6 +19618,7 @@  static int jit_subprogs(struct bpf_verifier_env *env)
 	prog->aux->real_func_cnt = env->subprog_cnt;
 	prog->aux->bpf_exception_cb = (void *)func[env->exception_callback_subprog]->bpf_func;
 	prog->aux->exception_boundary = func[0]->aux->exception_boundary;
+	prog->aux->fdtab = func[0]->aux->fdtab;
 	bpf_prog_jit_attempt_done(prog);
 	return 0;
 out_free: