diff mbox series

[bpf-next,07/29] bpf, x64: Allow to use caller address from stack

Message ID 20211118112455.475349-8-jolsa@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Add batch support for attaching trampolines | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next fail VM_Test
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 12429 this patch: 12429
netdev/cc_maintainers warning 10 maintainers not CCed: dsahern@kernel.org dave.hansen@linux.intel.com bp@alien8.de hpa@zytor.com yoshfuji@linux-ipv6.org kpsingh@kernel.org davem@davemloft.net tglx@linutronix.de mingo@redhat.com x86@kernel.org
netdev/build_clang success Errors and warnings before: 2101 this patch: 2101
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 11594 this patch: 11594
netdev/checkpatch warning WARNING: From:/Signed-off-by: email address mismatch: 'From: Jiri Olsa <jolsa@redhat.com>' != 'Signed-off-by: Jiri Olsa <jolsa@kernel.org>'
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiri Olsa Nov. 18, 2021, 11:24 a.m. UTC
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.

Adding support to retrieve the original function address from the stack
by adding new BPF_TRAMP_F_ORIG_STACK flag for arch_prepare_bpf_trampoline
function.

Basically we take the return address of the 'fentry' call:

   function + 0: call fentry    # stores 'function + 5' address on stack
   function + 5: ...

The 'function + 5' address will be used as the address for the
original function to call.

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(-)

Comments

Alexei Starovoitov Nov. 19, 2021, 4:14 a.m. UTC | #1
On Thu, Nov 18, 2021 at 12:24:33PM +0100, Jiri Olsa wrote:
> 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.
> 
> Adding support to retrieve the original function address from the stack
> by adding new BPF_TRAMP_F_ORIG_STACK flag for arch_prepare_bpf_trampoline
> function.
> 
> Basically we take the return address of the 'fentry' call:
> 
>    function + 0: call fentry    # stores 'function + 5' address on stack
>    function + 5: ...
> 
> The 'function + 5' address will be used as the address for the
> original function to call.
> 
> 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 67e8ac9aaf0d..d87001073033 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2035,10 +2035,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);
> +			EMIT2(0xff, 0xd0); /* call *rax */

Either return an eror if repoline is on
or use emit_indirect_jump().
Jiri Olsa Nov. 19, 2021, 9:46 p.m. UTC | #2
On Thu, Nov 18, 2021 at 08:14:09PM -0800, Alexei Starovoitov wrote:
> On Thu, Nov 18, 2021 at 12:24:33PM +0100, Jiri Olsa wrote:
> > 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.
> > 
> > Adding support to retrieve the original function address from the stack
> > by adding new BPF_TRAMP_F_ORIG_STACK flag for arch_prepare_bpf_trampoline
> > function.
> > 
> > Basically we take the return address of the 'fentry' call:
> > 
> >    function + 0: call fentry    # stores 'function + 5' address on stack
> >    function + 5: ...
> > 
> > The 'function + 5' address will be used as the address for the
> > original function to call.
> > 
> > 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 67e8ac9aaf0d..d87001073033 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -2035,10 +2035,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);
> > +			EMIT2(0xff, 0xd0); /* call *rax */
> 
> Either return an eror if repoline is on
> or use emit_indirect_jump().
> 

ok, will check

thanks,
jirka
diff mbox series

Patch

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 67e8ac9aaf0d..d87001073033 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2035,10 +2035,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);
+			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 cc7a0c36e7df..77c76e2fa9ff 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -594,6 +594,11 @@  struct btf_func_model {
 /* Return the return value of fentry prog. Only used by bpf_struct_ops. */
 #define BPF_TRAMP_F_RET_FENTRY_RET	BIT(4)
 
+/* Get original function from stack instead of from provided direct address.
+ * Makes sense for fexit programs only.
+ */
+#define BPF_TRAMP_F_ORIG_STACK		BIT(5)
+
 /* 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
  */