diff mbox series

[bpf-next,v4,11/14] libbpf: Fix accessing the first syscall argument on s390

Message ID 20220208051635.2160304-12-iii@linux.ibm.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Fix accessing syscall arguments | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
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 success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 6 maintainers not CCed: kpsingh@kernel.org john.fastabend@gmail.com kafai@fb.com songliubraving@fb.com yhs@fb.com netdev@vger.kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
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: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 29 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next fail VM_Test

Commit Message

Ilya Leoshkevich Feb. 8, 2022, 5:16 a.m. UTC
On s390, the first syscall argument should be accessed via orig_gpr2
(see arch/s390/include/asm/syscall.h). Currently gpr[2] is used
instead, leading to bpf_syscall_macro test failure.

Note that this is unfixable for CO-RE when vmlinux.h is not included.
Simply fail the build in this case.

Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tools/lib/bpf/bpf_tracing.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Ilya Leoshkevich Feb. 8, 2022, 1:10 p.m. UTC | #1
On Tue, 2022-02-08 at 06:16 +0100, Ilya Leoshkevich wrote:
> On s390, the first syscall argument should be accessed via orig_gpr2
> (see arch/s390/include/asm/syscall.h). Currently gpr[2] is used
> instead, leading to bpf_syscall_macro test failure.
> 
> Note that this is unfixable for CO-RE when vmlinux.h is not included.
> Simply fail the build in this case.
> 
> Reported-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  tools/lib/bpf/bpf_tracing.h | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/bpf_tracing.h
> b/tools/lib/bpf/bpf_tracing.h
> index 88ed5ba9510c..5911b177728f 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -2,6 +2,8 @@
>  #ifndef __BPF_TRACING_H__
>  #define __BPF_TRACING_H__
>  
> +#include <bpf/bpf_common_helpers.h>
> +
>  /* Scan the ARCH passed in from ARCH env variable (see Makefile) */
>  #if defined(__TARGET_ARCH_x86)
>         #define bpf_target_x86
> @@ -118,9 +120,20 @@
>  
>  #define __BPF_ARCH_HAS_SYSCALL_WRAPPER
>  
> -#if !defined(__KERNEL__) && !defined(__VMLINUX_H__)
> +#if defined(__KERNEL__) || defined(__VMLINUX_H__)
> +#define __PT_PARM1_REG_SYSCALL orig_gpr2
> +#else
>  /* s390 provides user_pt_regs instead of struct pt_regs to userspace
> */
>  #define __PT_REGS_CAST(x) ((const user_pt_regs *)(x))
> +/*
> + * struct pt_regs.orig_gpr2 is not exposed through user_pt_regs, and
> the ABI
> + * prohibits extending user_pt_regs. In non-CO-RE case, make use of
> the fact
> + * that orig_gpr2 comes right after gprs in struct pt_regs. CO-RE
> does not
> + * allow such hacks, so there is no way to access orig_gpr2.
> + */
> +#define PT_REGS_PARM1_SYSCALL(x) \
> +       (*(unsigned long *)(((char *)(x) + offsetofend(user_pt_regs,
> gprs))))
> +#define __PT_PARM1_REG_SYSCALL __unsupported__
>  #endif
>  
>  #define __PT_PARM1_REG gprs[2]

This might be too pessimistic.

While I don't see a way to add real CO-RE support here, and doing
something like __CORE_RELO(gprs, BYTE_OFFSET) + __CORE_RELO(gprs,
BYTE_SIZE) defeats the purpose of using CO-RE, we promise not to move
orig_gpr2 around. So just this:

#define PT_REGS_PARM1_CORE_SYSCALL(x) ({\
	unsigned long __tmp; \
	bpf_probe_read_kernel(&tmp, sizeof(__tmp),
&PT_REGS_PARM1_SYSCALL(x)); \
	tmp; \
})

does the trick here in a sense that, having been compiled once, it
would run everywhere. What do you think?
diff mbox series

Patch

diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 88ed5ba9510c..5911b177728f 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -2,6 +2,8 @@ 
 #ifndef __BPF_TRACING_H__
 #define __BPF_TRACING_H__
 
+#include <bpf/bpf_common_helpers.h>
+
 /* Scan the ARCH passed in from ARCH env variable (see Makefile) */
 #if defined(__TARGET_ARCH_x86)
 	#define bpf_target_x86
@@ -118,9 +120,20 @@ 
 
 #define __BPF_ARCH_HAS_SYSCALL_WRAPPER
 
-#if !defined(__KERNEL__) && !defined(__VMLINUX_H__)
+#if defined(__KERNEL__) || defined(__VMLINUX_H__)
+#define __PT_PARM1_REG_SYSCALL orig_gpr2
+#else
 /* s390 provides user_pt_regs instead of struct pt_regs to userspace */
 #define __PT_REGS_CAST(x) ((const user_pt_regs *)(x))
+/*
+ * struct pt_regs.orig_gpr2 is not exposed through user_pt_regs, and the ABI
+ * prohibits extending user_pt_regs. In non-CO-RE case, make use of the fact
+ * that orig_gpr2 comes right after gprs in struct pt_regs. CO-RE does not
+ * allow such hacks, so there is no way to access orig_gpr2.
+ */
+#define PT_REGS_PARM1_SYSCALL(x) \
+	(*(unsigned long *)(((char *)(x) + offsetofend(user_pt_regs, gprs))))
+#define __PT_PARM1_REG_SYSCALL __unsupported__
 #endif
 
 #define __PT_PARM1_REG gprs[2]