Re: [PATCH 09/19] bpf, x64: Allow to use caller address from stack

On Sun, Jun 06, 2021 at 08:07:44PM -0700, Yonghong Song wrote:

On 6/5/21 4:10 AM, Jiri Olsa wrote:

quoted

Currently we call the original function by using the absolute address
given at the JIT generation. That's not usable when having trampoline
attached to multiple functions. In this case we need to take the
return address from the stack.

Here, it is mentioned to take the return address from the stack.

quoted

Adding support to retrieve the original function address from the stack

Here, it is said to take original funciton address from the stack.

sorry if the description is confusing as always, the idea
is to take the function's return address from fentry call:

   function
     call fentry
     xxxx             <---- this address 

and use it to call the original function body before fexit handler

jirka

quoted

by adding new BPF_TRAMP_F_ORIG_STACK flag for arch_prepare_bpf_trampoline
function.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
  arch/x86/net/bpf_jit_comp.c | 13 +++++++++----
  include/linux/bpf.h         |  5 +++++
  2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 2a2e290fa5d8..b77e6bd78354 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c

@@ -2013,10 +2013,15 @@ int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image, void *i
  	if (flags & BPF_TRAMP_F_CALL_ORIG) {
  		restore_regs(m, &prog, nr_args, stack_size);
-		/* call original function */
-		if (emit_call(&prog, orig_call, prog)) {
-			ret = -EINVAL;
-			goto cleanup;
+		if (flags & BPF_TRAMP_F_ORIG_STACK) {
+			emit_ldx(&prog, BPF_DW, BPF_REG_0, BPF_REG_FP, 8);

This is load double from base_pointer + 8 which should be func return
address for x86, yet we try to call it.
I guess I must have missed something
here. Could you give some explanation?

quoted

+			EMIT2(0xff, 0xd0); /* call *rax */
+		} else {
+			/* call original function */
+			if (emit_call(&prog, orig_call, prog)) {
+				ret = -EINVAL;
+				goto cleanup;
+			}
  		}
  		/* remember return value in a stack for bpf prog to access */
  		emit_stx(&prog, BPF_DW, BPF_REG_FP, BPF_REG_0, -8);

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 86dec5001ae2..16fc600503fb 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h

@@ -554,6 +554,11 @@ struct btf_func_model {
   */
  #define BPF_TRAMP_F_SKIP_FRAME		BIT(2)
+/* Get original function from stack instead of from provided direct address.
+ * Makes sense for fexit programs only.
+ */
+#define BPF_TRAMP_F_ORIG_STACK		BIT(3)
+
  /* Each call __bpf_prog_enter + call bpf_func + call __bpf_prog_exit is ~50
   * bytes on x86.  Pick a number to fit into BPF_IMAGE_SIZE / 2
   */