* [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments
@ 2022-02-04 4:19 Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 01/10] arm64/bpf: Add orig_x0 to user_pt_regs Ilya Leoshkevich
` (10 more replies)
0 siblings, 11 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
libbpf now has macros to access syscall arguments in an
architecture-agnostic manner, but unfortunately they have a number of
issues on non-Intel arches, that this series aims to fix.
v1: https://lore.kernel.org/bpf/20220201234200.1836443-1-iii@linux.ibm.com/
v1 -> v2:
* Put orig_gpr2 in place of args[1] on s390 (Vasily).
* Fix arm64, powerpc and riscv (Heiko).
The arm64 fix is similar to the s390 one.
powerpc and riscv are different in that they unpack arguments to
registers before invoking syscall handlers - libbpf needs to know about
this difference, so I've decided to introduce PT_REGS_SYSCALL macro for
this (see bpf_syscall_macro test for usage example).
Tested in QEMU.
@Catalin, @Michael, @Paul: could you please review the arm64, powerpc
and riscv parts?
Ilya Leoshkevich (10):
arm64/bpf: Add orig_x0 to user_pt_regs
s390/bpf: Add orig_gpr2 to user_pt_regs
selftests/bpf: Fix an endianness issue in bpf_syscall_macro test
libbpf: Add __PT_PARM1_REG_SYSCALL macro
libbpf: Add PT_REGS_SYSCALL macro
selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro
libbpf: Fix accessing the first syscall argument on arm64
libbpf: Fix accessing syscall arguments on powerpc
libbpf: Fix accessing syscall arguments on riscv
libbpf: Fix accessing the first syscall argument on s390
arch/arm64/include/asm/ptrace.h | 2 +-
arch/arm64/include/uapi/asm/ptrace.h | 1 +
arch/s390/include/asm/ptrace.h | 3 +--
arch/s390/include/uapi/asm/ptrace.h | 2 +-
tools/lib/bpf/bpf_tracing.h | 23 ++++++++++++++++++-
.../selftests/bpf/progs/bpf_syscall_macro.c | 7 ++++--
6 files changed, 31 insertions(+), 7 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 01/10] arm64/bpf: Add orig_x0 to user_pt_regs
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 " Ilya Leoshkevich
` (9 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
orig_x0 is needed in order to access the first syscall argument from
eBPF programs.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
arch/arm64/include/asm/ptrace.h | 2 +-
arch/arm64/include/uapi/asm/ptrace.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 41b332c054ab..1be22f7870f8 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -183,9 +183,9 @@ struct pt_regs {
u64 sp;
u64 pc;
u64 pstate;
+ u64 orig_x0;
};
};
- u64 orig_x0;
#ifdef __AARCH64EB__
u32 unused2;
s32 syscallno;
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 758ae984ff97..3c118c5b0893 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -90,6 +90,7 @@ struct user_pt_regs {
__u64 sp;
__u64 pc;
__u64 pstate;
+ __u64 orig_x0;
};
struct user_fpsimd_state {
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 to user_pt_regs
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 01/10] arm64/bpf: Add orig_x0 to user_pt_regs Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 5:19 ` Andrii Nakryiko
2022-02-04 4:19 ` [PATCH bpf-next v2 03/10] selftests/bpf: Fix an endianness issue in bpf_syscall_macro test Ilya Leoshkevich
` (8 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
user_pt_regs is used by eBPF in order to access userspace registers -
see commit 466698e654e8 ("s390/bpf: correct broken uapi for
BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first
syscall argument from eBPF programs, we need to export orig_gpr2.
args member is not in use since commit 56e62a737028 ("s390: convert to
generic entry"), so move orig_gpr2 in its place.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
arch/s390/include/asm/ptrace.h | 3 +--
arch/s390/include/uapi/asm/ptrace.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
index 4ffa8e7f0ed3..0278bacd61be 100644
--- a/arch/s390/include/asm/ptrace.h
+++ b/arch/s390/include/asm/ptrace.h
@@ -80,12 +80,11 @@ struct pt_regs {
union {
user_pt_regs user_regs;
struct {
- unsigned long args[1];
+ unsigned long orig_gpr2;
psw_t psw;
unsigned long gprs[NUM_GPRS];
};
};
- unsigned long orig_gpr2;
union {
struct {
unsigned int int_code;
diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
index ad64d673b5e6..d0cc737b8151 100644
--- a/arch/s390/include/uapi/asm/ptrace.h
+++ b/arch/s390/include/uapi/asm/ptrace.h
@@ -292,7 +292,7 @@ typedef struct {
* the in-kernel pt_regs structure to user space.
*/
typedef struct {
- unsigned long args[1];
+ unsigned long orig_gpr2;
psw_t psw;
unsigned long gprs[NUM_GPRS];
} user_pt_regs;
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 03/10] selftests/bpf: Fix an endianness issue in bpf_syscall_macro test
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 01/10] arm64/bpf: Add orig_x0 to user_pt_regs Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 " Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 04/10] libbpf: Add __PT_PARM1_REG_SYSCALL macro Ilya Leoshkevich
` (7 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
bpf_syscall_macro reads a long argument into an int variable, which
produces a wrong value on big-endian systems. Fix by reading the
argument into an intermediate long variable first.
Fixes: 77fc0330dfe5 ("selftests/bpf: Add a test to confirm PT_REGS_PARM4_SYSCALL")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/progs/bpf_syscall_macro.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
index c8e60220cda8..f5c6ef2ff6d1 100644
--- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
@@ -28,6 +28,7 @@ int BPF_KPROBE(handle_sys_prctl)
{
struct pt_regs *real_regs;
pid_t pid = bpf_get_current_pid_tgid() >> 32;
+ unsigned long tmp;
if (pid != filter_pid)
return 0;
@@ -35,7 +36,9 @@ int BPF_KPROBE(handle_sys_prctl)
real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx);
/* test for PT_REGS_PARM */
- bpf_probe_read_kernel(&arg1, sizeof(arg1), &PT_REGS_PARM1_SYSCALL(real_regs));
+
+ bpf_probe_read_kernel(&tmp, sizeof(tmp), &PT_REGS_PARM1_SYSCALL(real_regs));
+ arg1 = tmp;
bpf_probe_read_kernel(&arg2, sizeof(arg2), &PT_REGS_PARM2_SYSCALL(real_regs));
bpf_probe_read_kernel(&arg3, sizeof(arg3), &PT_REGS_PARM3_SYSCALL(real_regs));
bpf_probe_read_kernel(&arg4_cx, sizeof(arg4_cx), &PT_REGS_PARM4(real_regs));
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 04/10] libbpf: Add __PT_PARM1_REG_SYSCALL macro
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (2 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 03/10] selftests/bpf: Fix an endianness issue in bpf_syscall_macro test Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro Ilya Leoshkevich
` (6 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
Some architectures have a special way to access the first syscall
argument. There already exists __PT_PARM4_REG_SYSCALL for the
fourth argument, so define a similar macro for the first one.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 032ba809f3e5..30f0964f8c9e 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -265,7 +265,11 @@ struct pt_regs;
#endif
+#ifdef __PT_PARM1_REG_SYSCALL
+#define PT_REGS_PARM1_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM1_REG_SYSCALL)
+#else /* __PT_PARM1_REG_SYSCALL */
#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
+#endif
#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
#ifdef __PT_PARM4_REG_SYSCALL
@@ -275,7 +279,11 @@ struct pt_regs;
#endif
#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
+#ifdef __PT_PARM1_REG_SYSCALL
+#define PT_REGS_PARM1_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM1_REG_SYSCALL)
+#else /* __PT_PARM1_REG_SYSCALL */
#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
+#endif
#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
#ifdef __PT_PARM4_REG_SYSCALL
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (3 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 04/10] libbpf: Add __PT_PARM1_REG_SYSCALL macro Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 5:22 ` Andrii Nakryiko
2022-02-04 4:19 ` [PATCH bpf-next v2 06/10] selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro Ilya Leoshkevich
` (5 subsequent siblings)
10 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
Some architectures pass a pointer to struct pt_regs to syscall
handlers, others unpack it into individual function parameters.
Introduce a macro to describe what a particular arch does, using
`passing pt_regs *` as a default.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 30f0964f8c9e..400a4f002f77 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -334,6 +334,15 @@ struct pt_regs;
#endif /* defined(bpf_target_defined) */
+/*
+ * When invoked from a syscall handler kprobe, returns a pointer to a
+ * struct pt_regs containing syscall arguments and suitable for passing to
+ * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
+ */
+#ifndef PT_REGS_SYSCALL
+#define PT_REGS_SYSCALL(ctx) ((struct pt_regs *)PT_REGS_PARM1(ctx))
+#endif
+
#ifndef ___bpf_concat
#define ___bpf_concat(a, b) a ## b
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 06/10] selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (4 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 07/10] libbpf: Fix accessing the first syscall argument on arm64 Ilya Leoshkevich
` (4 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
Ensure that PT_REGS_SYSCALL works correctly.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/progs/bpf_syscall_macro.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
index f5c6ef2ff6d1..826cc6e433de 100644
--- a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
+++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c
@@ -33,7 +33,7 @@ int BPF_KPROBE(handle_sys_prctl)
if (pid != filter_pid)
return 0;
- real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx);
+ real_regs = PT_REGS_SYSCALL(ctx);
/* test for PT_REGS_PARM */
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 07/10] libbpf: Fix accessing the first syscall argument on arm64
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (5 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 06/10] selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 08/10] libbpf: Fix accessing syscall arguments on powerpc Ilya Leoshkevich
` (3 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
On arm64, the first syscall argument should be accessed via orig_x0
(see arch/arm64/include/asm/syscall.h). Currently regs[0] is used
instead, leading to bpf_syscall_macro test failure.
Fixes: d084df3b7a4c ("libbpf: Fix the incorrect register read for syscalls on x86_64")
Reported-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 400a4f002f77..4a990111d9a8 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -145,6 +145,7 @@
/* arm64 provides struct user_pt_regs instead of struct pt_regs to userspace */
#define __PT_REGS_CAST(x) ((const struct user_pt_regs *)(x))
#define __PT_PARM1_REG regs[0]
+#define __PT_PARM1_REG_SYSCALL orig_x0
#define __PT_PARM2_REG regs[1]
#define __PT_PARM3_REG regs[2]
#define __PT_PARM4_REG regs[3]
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 08/10] libbpf: Fix accessing syscall arguments on powerpc
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (6 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 07/10] libbpf: Fix accessing the first syscall argument on arm64 Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv Ilya Leoshkevich
` (2 subsequent siblings)
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
powerpc's syscall handlers get "unpacked" arguments instead of a
struct pt_regs pointer. Indicate this to libbpf using PT_REGS_SYSCALL
macro.
Fixes: d084df3b7a4c ("libbpf: Fix the incorrect register read for syscalls on x86_64")
Reported-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 4a990111d9a8..c21aaecd711b 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -181,6 +181,7 @@
#define __PT_RC_REG gpr[3]
#define __PT_SP_REG sp
#define __PT_IP_REG nip
+#define PT_REGS_SYSCALL(ctx) ctx
#elif defined(bpf_target_sparc)
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (7 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 08/10] libbpf: Fix accessing syscall arguments on powerpc Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 5:25 ` Andrii Nakryiko
2022-02-04 4:19 ` [PATCH bpf-next v2 10/10] libbpf: Fix accessing the first syscall argument on s390 Ilya Leoshkevich
2022-02-04 5:28 ` [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Andrii Nakryiko
10 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich
riscv's syscall handlers get "unpacked" arguments instead of a
struct pt_regs pointer. Indicate this to libbpf using PT_REGS_SYSCALL
macro.
Fixes: d084df3b7a4c ("libbpf: Fix the incorrect register read for syscalls on x86_64")
Reported-by: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index c21aaecd711b..2b707aff0763 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -213,7 +213,8 @@
#define __PT_FP_REG fp
#define __PT_RC_REG a5
#define __PT_SP_REG sp
-#define __PT_IP_REG epc
+#define __PT_IP_REG pc
+#define PT_REGS_SYSCALL(ctx) ctx
#endif
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH bpf-next v2 10/10] libbpf: Fix accessing the first syscall argument on s390
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (8 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv Ilya Leoshkevich
@ 2022-02-04 4:19 ` Ilya Leoshkevich
2022-02-04 5:28 ` [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Andrii Nakryiko
10 siblings, 0 replies; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 4:19 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger,
Alexander Gordeev, Catalin Marinas, Michael Ellerman,
Paul Walmsley
Cc: bpf, Ilya Leoshkevich, Andrii Nakryiko
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.
Fixes: d084df3b7a4c ("libbpf: Fix the incorrect register read for syscalls on x86_64")
Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/lib/bpf/bpf_tracing.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index 2b707aff0763..cca62633f32c 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -117,6 +117,7 @@
/* s390 provides user_pt_regs instead of struct pt_regs to userspace */
#define __PT_REGS_CAST(x) ((const user_pt_regs *)(x))
#define __PT_PARM1_REG gprs[2]
+#define __PT_PARM1_REG_SYSCALL orig_gpr2
#define __PT_PARM2_REG gprs[3]
#define __PT_PARM3_REG gprs[4]
#define __PT_PARM4_REG gprs[5]
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 to user_pt_regs
2022-02-04 4:19 ` [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 " Ilya Leoshkevich
@ 2022-02-04 5:19 ` Andrii Nakryiko
2022-02-04 10:09 ` Heiko Carstens
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 5:19 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> user_pt_regs is used by eBPF in order to access userspace registers -
> see commit 466698e654e8 ("s390/bpf: correct broken uapi for
> BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first
> syscall argument from eBPF programs, we need to export orig_gpr2.
>
> args member is not in use since commit 56e62a737028 ("s390: convert to
> generic entry"), so move orig_gpr2 in its place.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> arch/s390/include/asm/ptrace.h | 3 +--
> arch/s390/include/uapi/asm/ptrace.h | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
> index 4ffa8e7f0ed3..0278bacd61be 100644
> --- a/arch/s390/include/asm/ptrace.h
> +++ b/arch/s390/include/asm/ptrace.h
> @@ -80,12 +80,11 @@ struct pt_regs {
> union {
> user_pt_regs user_regs;
> struct {
> - unsigned long args[1];
> + unsigned long orig_gpr2;
> psw_t psw;
> unsigned long gprs[NUM_GPRS];
> };
> };
> - unsigned long orig_gpr2;
Please don't change the physical location of this field, it
effectively breaks libbpf's syscall tracing macro on all older
kernels. Let's do what you did in the previous revision and just
expose the field at its correct offset. That way with up to date UAPI
header or vmlinux.h all this will work even on old kernels (even
without CO-RE).
> union {
> struct {
> unsigned int int_code;
> diff --git a/arch/s390/include/uapi/asm/ptrace.h b/arch/s390/include/uapi/asm/ptrace.h
> index ad64d673b5e6..d0cc737b8151 100644
> --- a/arch/s390/include/uapi/asm/ptrace.h
> +++ b/arch/s390/include/uapi/asm/ptrace.h
> @@ -292,7 +292,7 @@ typedef struct {
> * the in-kernel pt_regs structure to user space.
> */
> typedef struct {
> - unsigned long args[1];
> + unsigned long orig_gpr2;
> psw_t psw;
> unsigned long gprs[NUM_GPRS];
> } user_pt_regs;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro
2022-02-04 4:19 ` [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro Ilya Leoshkevich
@ 2022-02-04 5:22 ` Andrii Nakryiko
2022-02-04 5:23 ` Andrii Nakryiko
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 5:22 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> Some architectures pass a pointer to struct pt_regs to syscall
> handlers, others unpack it into individual function parameters.
> Introduce a macro to describe what a particular arch does, using
> `passing pt_regs *` as a default.
>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index 30f0964f8c9e..400a4f002f77 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -334,6 +334,15 @@ struct pt_regs;
>
> #endif /* defined(bpf_target_defined) */
>
> +/*
> + * When invoked from a syscall handler kprobe, returns a pointer to a
> + * struct pt_regs containing syscall arguments and suitable for passing to
> + * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
> + */
> +#ifndef PT_REGS_SYSCALL
> +#define PT_REGS_SYSCALL(ctx) ((struct pt_regs *)PT_REGS_PARM1(ctx))
> +#endif
maybe PT_REGS_SYSCALL_REGS? It returns regs, not the "syscall".
PT_REGS prefix is for consistency with all other pt_regs macros, but
"SYSCALL_REGS" is specifying what is actually returned by the macro
> +
> #ifndef ___bpf_concat
> #define ___bpf_concat(a, b) a ## b
> #endif
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro
2022-02-04 5:22 ` Andrii Nakryiko
@ 2022-02-04 5:23 ` Andrii Nakryiko
2022-02-04 12:29 ` Ilya Leoshkevich
0 siblings, 1 reply; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 5:23 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 3, 2022 at 9:22 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >
> > Some architectures pass a pointer to struct pt_regs to syscall
> > handlers, others unpack it into individual function parameters.
> > Introduce a macro to describe what a particular arch does, using
> > `passing pt_regs *` as a default.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> > index 30f0964f8c9e..400a4f002f77 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -334,6 +334,15 @@ struct pt_regs;
> >
> > #endif /* defined(bpf_target_defined) */
> >
> > +/*
> > + * When invoked from a syscall handler kprobe, returns a pointer to a
> > + * struct pt_regs containing syscall arguments and suitable for passing to
> > + * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
> > + */
> > +#ifndef PT_REGS_SYSCALL
> > +#define PT_REGS_SYSCALL(ctx) ((struct pt_regs *)PT_REGS_PARM1(ctx))
> > +#endif
>
> maybe PT_REGS_SYSCALL_REGS? It returns regs, not the "syscall".
> PT_REGS prefix is for consistency with all other pt_regs macros, but
> "SYSCALL_REGS" is specifying what is actually returned by the macro
>
Oh, and instead of casting to `struct pt_regs *` directly, maybe use
__PT_REGS_CAST() instead? For some architectures it probably should
stay user_pt_regs (or whatever it is there).
> > +
> > #ifndef ___bpf_concat
> > #define ___bpf_concat(a, b) a ## b
> > #endif
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv
2022-02-04 4:19 ` [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv Ilya Leoshkevich
@ 2022-02-04 5:25 ` Andrii Nakryiko
0 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 5:25 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> riscv's syscall handlers get "unpacked" arguments instead of a
> struct pt_regs pointer. Indicate this to libbpf using PT_REGS_SYSCALL
> macro.
>
> Fixes: d084df3b7a4c ("libbpf: Fix the incorrect register read for syscalls on x86_64")
This doesn't really fix that commit, it just adds an analogous fix for
a different architecture.
> Reported-by: Heiko Carstens <hca@linux.ibm.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> tools/lib/bpf/bpf_tracing.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index c21aaecd711b..2b707aff0763 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -213,7 +213,8 @@
> #define __PT_FP_REG fp
> #define __PT_RC_REG a5
> #define __PT_SP_REG sp
> -#define __PT_IP_REG epc
> +#define __PT_IP_REG pc
Is this epc -> pc change intentional? If yes, please split it into a
separate patch with corresponding Fixes tag and explanation.
> +#define PT_REGS_SYSCALL(ctx) ctx
>
> #endif
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
` (9 preceding siblings ...)
2022-02-04 4:19 ` [PATCH bpf-next v2 10/10] libbpf: Fix accessing the first syscall argument on s390 Ilya Leoshkevich
@ 2022-02-04 5:28 ` Andrii Nakryiko
10 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 5:28 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> libbpf now has macros to access syscall arguments in an
> architecture-agnostic manner, but unfortunately they have a number of
> issues on non-Intel arches, that this series aims to fix.
>
> v1: https://lore.kernel.org/bpf/20220201234200.1836443-1-iii@linux.ibm.com/
> v1 -> v2:
> * Put orig_gpr2 in place of args[1] on s390 (Vasily).
> * Fix arm64, powerpc and riscv (Heiko).
>
> The arm64 fix is similar to the s390 one.
>
> powerpc and riscv are different in that they unpack arguments to
> registers before invoking syscall handlers - libbpf needs to know about
> this difference, so I've decided to introduce PT_REGS_SYSCALL macro for
> this (see bpf_syscall_macro test for usage example).
>
> Tested in QEMU.
>
> @Catalin, @Michael, @Paul: could you please review the arm64, powerpc
> and riscv parts?
I think it's worth waiting for these fixes before cutting libbpf v0.7
release, thanks for working on them! Please see my other comments on
respective patches.
>
> Ilya Leoshkevich (10):
> arm64/bpf: Add orig_x0 to user_pt_regs
> s390/bpf: Add orig_gpr2 to user_pt_regs
> selftests/bpf: Fix an endianness issue in bpf_syscall_macro test
> libbpf: Add __PT_PARM1_REG_SYSCALL macro
> libbpf: Add PT_REGS_SYSCALL macro
> selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro
> libbpf: Fix accessing the first syscall argument on arm64
> libbpf: Fix accessing syscall arguments on powerpc
> libbpf: Fix accessing syscall arguments on riscv
> libbpf: Fix accessing the first syscall argument on s390
>
> arch/arm64/include/asm/ptrace.h | 2 +-
> arch/arm64/include/uapi/asm/ptrace.h | 1 +
> arch/s390/include/asm/ptrace.h | 3 +--
> arch/s390/include/uapi/asm/ptrace.h | 2 +-
> tools/lib/bpf/bpf_tracing.h | 23 ++++++++++++++++++-
> .../selftests/bpf/progs/bpf_syscall_macro.c | 7 ++++--
> 6 files changed, 31 insertions(+), 7 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 to user_pt_regs
2022-02-04 5:19 ` Andrii Nakryiko
@ 2022-02-04 10:09 ` Heiko Carstens
0 siblings, 0 replies; 19+ messages in thread
From: Heiko Carstens @ 2022-02-04 10:09 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Ilya Leoshkevich, Alexei Starovoitov, Daniel Borkmann,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, Feb 03, 2022 at 09:19:42PM -0800, Andrii Nakryiko wrote:
> On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >
> > user_pt_regs is used by eBPF in order to access userspace registers -
> > see commit 466698e654e8 ("s390/bpf: correct broken uapi for
> > BPF_PROG_TYPE_PERF_EVENT program type"). In order to access the first
> > syscall argument from eBPF programs, we need to export orig_gpr2.
> >
> > args member is not in use since commit 56e62a737028 ("s390: convert to
> > generic entry"), so move orig_gpr2 in its place.
> >
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> > arch/s390/include/asm/ptrace.h | 3 +--
> > arch/s390/include/uapi/asm/ptrace.h | 2 +-
> > 2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/s390/include/asm/ptrace.h b/arch/s390/include/asm/ptrace.h
> > index 4ffa8e7f0ed3..0278bacd61be 100644
> > --- a/arch/s390/include/asm/ptrace.h
> > +++ b/arch/s390/include/asm/ptrace.h
> > @@ -80,12 +80,11 @@ struct pt_regs {
> > union {
> > user_pt_regs user_regs;
> > struct {
> > - unsigned long args[1];
> > + unsigned long orig_gpr2;
> > psw_t psw;
> > unsigned long gprs[NUM_GPRS];
> > };
> > };
> > - unsigned long orig_gpr2;
>
> Please don't change the physical location of this field, it
> effectively breaks libbpf's syscall tracing macro on all older
> kernels. Let's do what you did in the previous revision and just
> expose the field at its correct offset. That way with up to date UAPI
> header or vmlinux.h all this will work even on old kernels (even
> without CO-RE).
That's (unfortunately) a valid argument. So looks like we can't get rid of
the args member now. Maybe we can put something else there or simply rename
it to "dontuse".
Anyway, that's not within the scope of this patch series.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro
2022-02-04 5:23 ` Andrii Nakryiko
@ 2022-02-04 12:29 ` Ilya Leoshkevich
2022-02-04 18:09 ` Andrii Nakryiko
0 siblings, 1 reply; 19+ messages in thread
From: Ilya Leoshkevich @ 2022-02-04 12:29 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Thu, 2022-02-03 at 21:23 -0800, Andrii Nakryiko wrote:
> On Thu, Feb 3, 2022 at 9:22 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com>
> > wrote:
> > >
> > > Some architectures pass a pointer to struct pt_regs to syscall
> > > handlers, others unpack it into individual function parameters.
> > > Introduce a macro to describe what a particular arch does, using
> > > `passing pt_regs *` as a default.
> > >
> > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > ---
> > > tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > b/tools/lib/bpf/bpf_tracing.h
> > > index 30f0964f8c9e..400a4f002f77 100644
> > > --- a/tools/lib/bpf/bpf_tracing.h
> > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > @@ -334,6 +334,15 @@ struct pt_regs;
> > >
> > > #endif /* defined(bpf_target_defined) */
> > >
> > > +/*
> > > + * When invoked from a syscall handler kprobe, returns a pointer
> > > to a
> > > + * struct pt_regs containing syscall arguments and suitable for
> > > passing to
> > > + * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
> > > + */
> > > +#ifndef PT_REGS_SYSCALL
> > > +#define PT_REGS_SYSCALL(ctx) ((struct pt_regs
> > > *)PT_REGS_PARM1(ctx))
> > > +#endif
> >
> > maybe PT_REGS_SYSCALL_REGS? It returns regs, not the "syscall".
> > PT_REGS prefix is for consistency with all other pt_regs macros,
> > but
> > "SYSCALL_REGS" is specifying what is actually returned by the macro
> >
>
> Oh, and instead of casting to `struct pt_regs *` directly, maybe use
> __PT_REGS_CAST() instead? For some architectures it probably should
> stay user_pt_regs (or whatever it is there).
>
> > > +
> > > #ifndef ___bpf_concat
> > > #define ___bpf_concat(a, b) a ## b
> > > #endif
> > > --
> > > 2.34.1
> > >
I think it's better to keep this as struct pt_regs *, so that in
bpf progs we can do
struct pt_regs *real_regs = PT_REGS_SYSCALL(ctx);
without having to worry about which arch we are on, or using the
opaque void *.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro
2022-02-04 12:29 ` Ilya Leoshkevich
@ 2022-02-04 18:09 ` Andrii Nakryiko
0 siblings, 0 replies; 19+ messages in thread
From: Andrii Nakryiko @ 2022-02-04 18:09 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, Alexander Gordeev,
Catalin Marinas, Michael Ellerman, Paul Walmsley, bpf
On Fri, Feb 4, 2022 at 4:30 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>
> On Thu, 2022-02-03 at 21:23 -0800, Andrii Nakryiko wrote:
> > On Thu, Feb 3, 2022 at 9:22 PM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> > >
> > > On Thu, Feb 3, 2022 at 8:20 PM Ilya Leoshkevich <iii@linux.ibm.com>
> > > wrote:
> > > >
> > > > Some architectures pass a pointer to struct pt_regs to syscall
> > > > handlers, others unpack it into individual function parameters.
> > > > Introduce a macro to describe what a particular arch does, using
> > > > `passing pt_regs *` as a default.
> > > >
> > > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > > > ---
> > > > tools/lib/bpf/bpf_tracing.h | 9 +++++++++
> > > > 1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > > b/tools/lib/bpf/bpf_tracing.h
> > > > index 30f0964f8c9e..400a4f002f77 100644
> > > > --- a/tools/lib/bpf/bpf_tracing.h
> > > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > > @@ -334,6 +334,15 @@ struct pt_regs;
> > > >
> > > > #endif /* defined(bpf_target_defined) */
> > > >
> > > > +/*
> > > > + * When invoked from a syscall handler kprobe, returns a pointer
> > > > to a
> > > > + * struct pt_regs containing syscall arguments and suitable for
> > > > passing to
> > > > + * PT_REGS_PARMn_SYSCALL() and PT_REGS_PARMn_CORE_SYSCALL().
> > > > + */
> > > > +#ifndef PT_REGS_SYSCALL
> > > > +#define PT_REGS_SYSCALL(ctx) ((struct pt_regs
> > > > *)PT_REGS_PARM1(ctx))
> > > > +#endif
> > >
> > > maybe PT_REGS_SYSCALL_REGS? It returns regs, not the "syscall".
> > > PT_REGS prefix is for consistency with all other pt_regs macros,
> > > but
> > > "SYSCALL_REGS" is specifying what is actually returned by the macro
> > >
> >
> > Oh, and instead of casting to `struct pt_regs *` directly, maybe use
> > __PT_REGS_CAST() instead? For some architectures it probably should
> > stay user_pt_regs (or whatever it is there).
> >
> > > > +
> > > > #ifndef ___bpf_concat
> > > > #define ___bpf_concat(a, b) a ## b
> > > > #endif
> > > > --
> > > > 2.34.1
> > > >
>
> I think it's better to keep this as struct pt_regs *, so that in
> bpf progs we can do
>
> struct pt_regs *real_regs = PT_REGS_SYSCALL(ctx);
>
> without having to worry about which arch we are on, or using the
> opaque void *.
Makes sense, sounds good to me.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-02-04 18:09 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-04 4:19 [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 01/10] arm64/bpf: Add orig_x0 to user_pt_regs Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 02/10] s390/bpf: Add orig_gpr2 " Ilya Leoshkevich
2022-02-04 5:19 ` Andrii Nakryiko
2022-02-04 10:09 ` Heiko Carstens
2022-02-04 4:19 ` [PATCH bpf-next v2 03/10] selftests/bpf: Fix an endianness issue in bpf_syscall_macro test Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 04/10] libbpf: Add __PT_PARM1_REG_SYSCALL macro Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 05/10] libbpf: Add PT_REGS_SYSCALL macro Ilya Leoshkevich
2022-02-04 5:22 ` Andrii Nakryiko
2022-02-04 5:23 ` Andrii Nakryiko
2022-02-04 12:29 ` Ilya Leoshkevich
2022-02-04 18:09 ` Andrii Nakryiko
2022-02-04 4:19 ` [PATCH bpf-next v2 06/10] selftests/bpf: Use PT_REGS_SYSCALL in bpf_syscall_macro Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 07/10] libbpf: Fix accessing the first syscall argument on arm64 Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 08/10] libbpf: Fix accessing syscall arguments on powerpc Ilya Leoshkevich
2022-02-04 4:19 ` [PATCH bpf-next v2 09/10] libbpf: Fix accessing syscall arguments on riscv Ilya Leoshkevich
2022-02-04 5:25 ` Andrii Nakryiko
2022-02-04 4:19 ` [PATCH bpf-next v2 10/10] libbpf: Fix accessing the first syscall argument on s390 Ilya Leoshkevich
2022-02-04 5:28 ` [PATCH bpf-next v2 00/10] libbpf: Fix accessing syscall arguments Andrii Nakryiko
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.