[PATCH bpf-next] s390/bpf: Fix indirect trampoline generation

[PATCH bpf-next] s390/bpf: Fix indirect trampoline generation

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

The func_addr used to be NULL for indirect trampolines used by struct_ops.
Now func_addr is a valid function pointer.
Hence use BPF_TRAMP_F_INDIRECT flag to detect such condition.

Fixes: 2cd3e3772e41 ("x86/cfi,bpf: Fix bpf_struct_ops CFI")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/s390/net/bpf_jit_comp.c               | 3 ++-
 tools/testing/selftests/bpf/DENYLIST.s390x | 2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index cc129617480a..7f0a7b97ef4c 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -2362,7 +2362,8 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 		return -ENOTSUPP;
 
 	/* Return to %r14, since func_addr and %r0 are not available. */
-	if (!func_addr && !(flags & BPF_TRAMP_F_ORIG_STACK))
+	if ((!func_addr && !(flags & BPF_TRAMP_F_ORIG_STACK)) ||
+	    (flags & BPF_TRAMP_F_INDIRECT))
 		flags |= BPF_TRAMP_F_SKIP_FRAME;
 
 	/*
diff --git a/tools/testing/selftests/bpf/DENYLIST.s390x b/tools/testing/selftests/bpf/DENYLIST.s390x
index d27aa42d11a4..1a63996c0304 100644
--- a/tools/testing/selftests/bpf/DENYLIST.s390x
+++ b/tools/testing/selftests/bpf/DENYLIST.s390x
@@ -1,7 +1,5 @@
 # TEMPORARY
 # Alphabetical order
-dummy_st_ops/dummy_init_ret_value
-dummy_st_ops/dummy_init_ptr_arg
 exceptions				 # JIT does not support calling kfunc bpf_throw				       (exceptions)
 get_stack_raw_tp                         # user_stack corrupted user stack                                             (no backchain userspace)
 stacktrace_build_id                      # compare_map_keys stackid_hmap vs. stackmap err -2 errno 2                   (?)
-- 
2.34.1

Re: [PATCH bpf-next] s390/bpf: Fix indirect trampoline generation

[Ilya Leoshkevich]

On Fri, Dec 15, 2023 at 04:45:49PM -0800, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> The func_addr used to be NULL for indirect trampolines used by struct_ops.
> Now func_addr is a valid function pointer.
> Hence use BPF_TRAMP_F_INDIRECT flag to detect such condition.
> 
> Fixes: 2cd3e3772e41 ("x86/cfi,bpf: Fix bpf_struct_ops CFI")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  arch/s390/net/bpf_jit_comp.c               | 3 ++-
>  tools/testing/selftests/bpf/DENYLIST.s390x | 2 --
>  2 files changed, 2 insertions(+), 3 deletions(-)

IIUC F_INDIRECT trampolines are called via C function pointers, and
func_addr does not participate in any call chains, but is rather used
as a source of CFI information. So returning to %r14 is the right
thing to do.

Thanks!

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Re: [PATCH bpf-next] s390/bpf: Fix indirect trampoline generation

[Alexei Starovoitov]

On Fri, Dec 15, 2023 at 5:56 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Fri, Dec 15, 2023 at 04:45:49PM -0800, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > The func_addr used to be NULL for indirect trampolines used by struct_ops.
> > Now func_addr is a valid function pointer.
> > Hence use BPF_TRAMP_F_INDIRECT flag to detect such condition.
> >
> > Fixes: 2cd3e3772e41 ("x86/cfi,bpf: Fix bpf_struct_ops CFI")
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> >  arch/s390/net/bpf_jit_comp.c               | 3 ++-
> >  tools/testing/selftests/bpf/DENYLIST.s390x | 2 --
> >  2 files changed, 2 insertions(+), 3 deletions(-)
>
> IIUC F_INDIRECT trampolines are called via C function pointers, and
> func_addr does not participate in any call chains, but is rather used
> as a source of CFI information.

Yes.

> So returning to %r14 is the right
> thing to do.
>
> Thanks!
>
> Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>

Thank you for the quick review.

Re: [PATCH bpf-next] s390/bpf: Fix indirect trampoline generation

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Fri, 15 Dec 2023 16:45:49 -0800 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> The func_addr used to be NULL for indirect trampolines used by struct_ops.
> Now func_addr is a valid function pointer.
> Hence use BPF_TRAMP_F_INDIRECT flag to detect such condition.
> 
> Fixes: 2cd3e3772e41 ("x86/cfi,bpf: Fix bpf_struct_ops CFI")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> 
> [...]

Here is the summary with links:
  - [bpf-next] s390/bpf: Fix indirect trampoline generation
    https://git.kernel.org/bpf/bpf-next/c/0c970ed2f87c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html