* [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
@ 2021-12-21 11:21 Kenta.Tada
2021-12-21 15:50 ` Yonghong Song
0 siblings, 1 reply; 8+ messages in thread
From: Kenta.Tada @ 2021-12-21 11:21 UTC (permalink / raw)
To: andrii, bpf
Cc: ast, daniel, kafai, songliubraving, yhs, john.fastabend, kpsingh
Currently, rcx is read as the fourth parameter of syscall on x86_64.
But x86_64 Linux System Call convention uses r10 actually.
This commit adds the wrapper for users who want to access to
syscall params to analyze the user space.
Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
---
tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index db05a5937105..f6fcccd9b10c 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -67,10 +67,15 @@
#if defined(__KERNEL__) || defined(__VMLINUX_H__)
#define PT_REGS_PARM1(x) ((x)->di)
+#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
#define PT_REGS_PARM2(x) ((x)->si)
+#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
#define PT_REGS_PARM3(x) ((x)->dx)
+#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
#define PT_REGS_PARM4(x) ((x)->cx)
+#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
#define PT_REGS_RET(x) ((x)->sp)
#define PT_REGS_FP(x) ((x)->bp)
#define PT_REGS_RC(x) ((x)->ax)
@@ -78,10 +83,15 @@
#define PT_REGS_IP(x) ((x)->ip)
#define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
+#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
#define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
+#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
#define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
+#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
#define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
+#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
#define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
+#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
#define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
#define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
#define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax)
@@ -117,10 +127,15 @@
#else
#define PT_REGS_PARM1(x) ((x)->rdi)
+#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
#define PT_REGS_PARM2(x) ((x)->rsi)
+#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
#define PT_REGS_PARM3(x) ((x)->rdx)
+#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
#define PT_REGS_PARM4(x) ((x)->rcx)
+#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
#define PT_REGS_PARM5(x) ((x)->r8)
+#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
#define PT_REGS_RET(x) ((x)->rsp)
#define PT_REGS_FP(x) ((x)->rbp)
#define PT_REGS_RC(x) ((x)->rax)
@@ -128,10 +143,15 @@
#define PT_REGS_IP(x) ((x)->rip)
#define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
+#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
#define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
+#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
#define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
+#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
#define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
+#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
#define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
+#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
#define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
#define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
#define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
--
2.32.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-21 11:21 [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta.Tada
@ 2021-12-21 15:50 ` Yonghong Song
2021-12-21 22:58 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Yonghong Song @ 2021-12-21 15:50 UTC (permalink / raw)
To: Kenta.Tada, andrii, bpf
Cc: ast, daniel, kafai, songliubraving, john.fastabend, kpsingh
On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> Currently, rcx is read as the fourth parameter of syscall on x86_64.
> But x86_64 Linux System Call convention uses r10 actually.
> This commit adds the wrapper for users who want to access to
> syscall params to analyze the user space.
>
> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> ---
> tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index db05a5937105..f6fcccd9b10c 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -67,10 +67,15 @@
> #if defined(__KERNEL__) || defined(__VMLINUX_H__)
>
> #define PT_REGS_PARM1(x) ((x)->di)
> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> #define PT_REGS_PARM2(x) ((x)->si)
> +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> #define PT_REGS_PARM3(x) ((x)->dx)
> +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> #define PT_REGS_PARM4(x) ((x)->cx)
> +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
I think this is correct. We have a bcc commit doing similar thing.
https://github.com/iovisor/bcc/commit/c23448e34ecd3cc9bfc19f0b43f4325f77c2e4cc#diff-c78ffb58f59e85eaba9bf9977b7202f3e50f17e2a9ee556c36a311f9a9ab5d6e
> #define PT_REGS_PARM5(x) ((x)->r8)
> +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> #define PT_REGS_RET(x) ((x)->sp)
> #define PT_REGS_FP(x) ((x)->bp)
> #define PT_REGS_RC(x) ((x)->ax)
> @@ -78,10 +83,15 @@
> #define PT_REGS_IP(x) ((x)->ip)
>
> #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
> #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax)
> @@ -117,10 +127,15 @@
> #else
>
> #define PT_REGS_PARM1(x) ((x)->rdi)
> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> #define PT_REGS_PARM2(x) ((x)->rsi)
> +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> #define PT_REGS_PARM3(x) ((x)->rdx)
> +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> #define PT_REGS_PARM4(x) ((x)->rcx)
> +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> #define PT_REGS_PARM5(x) ((x)->r8)
> +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> #define PT_REGS_RET(x) ((x)->rsp)
> #define PT_REGS_FP(x) ((x)->rbp)
> #define PT_REGS_RC(x) ((x)->rax)
> @@ -128,10 +143,15 @@
> #define PT_REGS_IP(x) ((x)->rip)
>
> #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
> #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
Looks like macros only available for x86_64. Can we make it also
available for other architectures so we won't introduce arch specific
codes into bpf program?
Also, could you add a selftest to use this macro, esp. for parameter 4?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-21 15:50 ` Yonghong Song
@ 2021-12-21 22:58 ` Andrii Nakryiko
2021-12-22 5:51 ` Kenta.Tada
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2021-12-21 22:58 UTC (permalink / raw)
To: Yonghong Song
Cc: Kenta.Tada, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, john fastabend, KP Singh
On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > But x86_64 Linux System Call convention uses r10 actually.
> > This commit adds the wrapper for users who want to access to
> > syscall params to analyze the user space.
> >
> > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > ---
> > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> > index db05a5937105..f6fcccd9b10c 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -67,10 +67,15 @@
> > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> >
> > #define PT_REGS_PARM1(x) ((x)->di)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->si)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->dx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->cx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
>
> I think this is correct. We have a bcc commit doing similar thing.
> https://github.com/iovisor/bcc/commit/c23448e34ecd3cc9bfc19f0b43f4325f77c2e4cc#diff-c78ffb58f59e85eaba9bf9977b7202f3e50f17e2a9ee556c36a311f9a9ab5d6e
>
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->sp)
> > #define PT_REGS_FP(x) ((x)->bp)
> > #define PT_REGS_RC(x) ((x)->ax)
> > @@ -78,10 +83,15 @@
> > #define PT_REGS_IP(x) ((x)->ip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax)
> > @@ -117,10 +127,15 @@
> > #else
> >
> > #define PT_REGS_PARM1(x) ((x)->rdi)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->rsi)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->rdx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->rcx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->rsp)
> > #define PT_REGS_FP(x) ((x)->rbp)
> > #define PT_REGS_RC(x) ((x)->rax)
> > @@ -128,10 +143,15 @@
> > #define PT_REGS_IP(x) ((x)->rip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /* syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
>
> Looks like macros only available for x86_64. Can we make it also
> available for other architectures so we won't introduce arch specific
> codes into bpf program?
Yeah, but instead of copy/pasting it for each architecture, let's
define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
arch with such inconsistency?) and then after all the architectures
defined their macro define
#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
...
#ifndef PT_REGS_PARM4_SYSCALL(x)
#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
#endif
That way we'll avoid all the extra "no-op" definitions.
>
> Also, could you add a selftest to use this macro, esp. for parameter 4?
+1
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-21 22:58 ` Andrii Nakryiko
@ 2021-12-22 5:51 ` Kenta.Tada
2021-12-22 6:19 ` Kenta.Tada
0 siblings, 1 reply; 8+ messages in thread
From: Kenta.Tada @ 2021-12-22 5:51 UTC (permalink / raw)
To: andrii.nakryiko, yhs
Cc: andrii, bpf, ast, daniel, kafai, songliubraving, john.fastabend, kpsingh
>> Looks like macros only available for x86_64. Can we make it also
>> available for other architectures so we won't introduce arch specific
>> codes into bpf program?
>
>Yeah, but instead of copy/pasting it for each architecture, let's define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only arch with such inconsistency?) and then after all the architectures defined their macro define
Currently, I think x86_64 is the only arch which causes this issue.
But I'll add #ifndef to not only the 4th parameter but also all parameters for future extensibility.
>>
>> Also, could you add a selftest to use this macro, esp. for parameter 4?
Sure.
I'll add the same macro to bpf_tracing.h in selftests.
Thanks!
-----Original Message-----
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Sent: Wednesday, December 22, 2021 7:58 AM
To: Yonghong Song <yhs@fb.com>
Cc: Tada, Kenta (SGC) <Kenta.Tada@sony.com>; Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
Subject: Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > But x86_64 Linux System Call convention uses r10 actually.
> > This commit adds the wrapper for users who want to access to syscall
> > params to analyze the user space.
> >
> > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > ---
> > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h
> > b/tools/lib/bpf/bpf_tracing.h index db05a5937105..f6fcccd9b10c
> > 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -67,10 +67,15 @@
> > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> >
> > #define PT_REGS_PARM1(x) ((x)->di)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->si)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->dx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->cx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
>
> I think this is correct. We have a bcc commit doing similar thing.
> https://urldefense.com/v3/__https://github.com/iovisor/bcc/commit/c234
> 48e34ecd3cc9bfc19f0b43f4325f77c2e4cc*diff-c78ffb58f59e85eaba9bf9977b72
> 02f3e50f17e2a9ee556c36a311f9a9ab5d6e__;Iw!!JmoZiZGBv3RvKRSx!qa2pOQlUSd
> WfoxmZ7EuPvZpWAlK5IBVP1eC-2d3njBLe3yehAXyV_0wI9mGRF5Q$ [github[.]com]
>
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->sp)
> > #define PT_REGS_FP(x) ((x)->bp)
> > #define PT_REGS_RC(x) ((x)->ax)
> > @@ -78,10 +83,15 @@
> > #define PT_REGS_IP(x) ((x)->ip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > +syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax) @@ -117,10
> > +127,15 @@
> > #else
> >
> > #define PT_REGS_PARM1(x) ((x)->rdi)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->rsi)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->rdx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->rcx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->rsp)
> > #define PT_REGS_FP(x) ((x)->rbp)
> > #define PT_REGS_RC(x) ((x)->rax)
> > @@ -128,10 +143,15 @@
> > #define PT_REGS_IP(x) ((x)->rip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > +syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
>
> Looks like macros only available for x86_64. Can we make it also
> available for other architectures so we won't introduce arch specific
> codes into bpf program?
Yeah, but instead of copy/pasting it for each architecture, let's define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only arch with such inconsistency?) and then after all the architectures defined their macro define
#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x) ...
#ifndef PT_REGS_PARM4_SYSCALL(x)
#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x) #endif
That way we'll avoid all the extra "no-op" definitions.
>
> Also, could you add a selftest to use this macro, esp. for parameter 4?
+1
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-22 5:51 ` Kenta.Tada
@ 2021-12-22 6:19 ` Kenta.Tada
2021-12-22 6:47 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Kenta.Tada @ 2021-12-22 6:19 UTC (permalink / raw)
To: andrii.nakryiko, yhs
Cc: andrii, bpf, ast, daniel, kafai, songliubraving, john.fastabend, kpsingh
>>
>> Also, could you add a selftest to use this macro, esp. for parameter 4?
I misunderstood this comment.
I'll just add a test of this new macro.
-----Original Message-----
From: Tada, Kenta (SGC)
Sent: Wednesday, December 22, 2021 2:52 PM
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>; Yonghong Song <yhs@fb.com>
Cc: Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
Subject: RE: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
>> Looks like macros only available for x86_64. Can we make it also
>> available for other architectures so we won't introduce arch specific
>> codes into bpf program?
>
>Yeah, but instead of copy/pasting it for each architecture, let's
>define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
>arch with such inconsistency?) and then after all the architectures
>defined their macro define
Currently, I think x86_64 is the only arch which causes this issue.
But I'll add #ifndef to not only the 4th parameter but also all parameters for future extensibility.
>>
>> Also, could you add a selftest to use this macro, esp. for parameter 4?
Sure.
I'll add the same macro to bpf_tracing.h in selftests.
Thanks!
-----Original Message-----
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Sent: Wednesday, December 22, 2021 7:58 AM
To: Yonghong Song <yhs@fb.com>
Cc: Tada, Kenta (SGC) <Kenta.Tada@sony.com>; Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
Subject: Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > But x86_64 Linux System Call convention uses r10 actually.
> > This commit adds the wrapper for users who want to access to syscall
> > params to analyze the user space.
> >
> > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > ---
> > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/tools/lib/bpf/bpf_tracing.h
> > b/tools/lib/bpf/bpf_tracing.h index db05a5937105..f6fcccd9b10c
> > 100644
> > --- a/tools/lib/bpf/bpf_tracing.h
> > +++ b/tools/lib/bpf/bpf_tracing.h
> > @@ -67,10 +67,15 @@
> > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> >
> > #define PT_REGS_PARM1(x) ((x)->di)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->si)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->dx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->cx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
>
> I think this is correct. We have a bcc commit doing similar thing.
> https://urldefense.com/v3/__https://github.com/iovisor/bcc/commit/c234
> 48e34ecd3cc9bfc19f0b43f4325f77c2e4cc*diff-c78ffb58f59e85eaba9bf9977b72
> 02f3e50f17e2a9ee556c36a311f9a9ab5d6e__;Iw!!JmoZiZGBv3RvKRSx!qa2pOQlUSd
> WfoxmZ7EuPvZpWAlK5IBVP1eC-2d3njBLe3yehAXyV_0wI9mGRF5Q$ [github[.]com]
>
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->sp)
> > #define PT_REGS_FP(x) ((x)->bp)
> > #define PT_REGS_RC(x) ((x)->ax)
> > @@ -78,10 +83,15 @@
> > #define PT_REGS_IP(x) ((x)->ip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > +syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax) @@ -117,10
> > +127,15 @@
> > #else
> >
> > #define PT_REGS_PARM1(x) ((x)->rdi)
> > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > #define PT_REGS_PARM2(x) ((x)->rsi)
> > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > #define PT_REGS_PARM3(x) ((x)->rdx)
> > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > #define PT_REGS_PARM4(x) ((x)->rcx)
> > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> > #define PT_REGS_PARM5(x) ((x)->r8)
> > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > #define PT_REGS_RET(x) ((x)->rsp)
> > #define PT_REGS_FP(x) ((x)->rbp)
> > #define PT_REGS_RC(x) ((x)->rax)
> > @@ -128,10 +143,15 @@
> > #define PT_REGS_IP(x) ((x)->rip)
> >
> > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > +syscall uses r10 */
> > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
>
> Looks like macros only available for x86_64. Can we make it also
> available for other architectures so we won't introduce arch specific
> codes into bpf program?
Yeah, but instead of copy/pasting it for each architecture, let's define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only arch with such inconsistency?) and then after all the architectures defined their macro define
#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x) ...
#ifndef PT_REGS_PARM4_SYSCALL(x)
#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x) #endif
That way we'll avoid all the extra "no-op" definitions.
>
> Also, could you add a selftest to use this macro, esp. for parameter 4?
+1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-22 6:19 ` Kenta.Tada
@ 2021-12-22 6:47 ` Andrii Nakryiko
2021-12-22 6:57 ` Kenta.Tada
0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2021-12-22 6:47 UTC (permalink / raw)
To: Kenta.Tada
Cc: Yonghong Song, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, john fastabend, KP Singh
On Tue, Dec 21, 2021 at 10:20 PM <Kenta.Tada@sony.com> wrote:
>
> >>
> >> Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> I misunderstood this comment.
> I'll just add a test of this new macro.
>
> -----Original Message-----
> From: Tada, Kenta (SGC)
> Sent: Wednesday, December 22, 2021 2:52 PM
> To: Andrii Nakryiko <andrii.nakryiko@gmail.com>; Yonghong Song <yhs@fb.com>
> Cc: Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
> Subject: RE: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
>
> >> Looks like macros only available for x86_64. Can we make it also
> >> available for other architectures so we won't introduce arch specific
> >> codes into bpf program?
> >
> >Yeah, but instead of copy/pasting it for each architecture, let's
> >define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
> >arch with such inconsistency?) and then after all the architectures
> >defined their macro define
>
> Currently, I think x86_64 is the only arch which causes this issue.
> But I'll add #ifndef to not only the 4th parameter but also all parameters for future extensibility.
Let's not add unnecessary #ifndefs. If we get a case for other args,
we'll add #ifndefs as necessary.
But after looking at your and Hengqi's patches in the last few days,
I've looked at the current state of bpf_tracing.h and felt like there
is too much repetition. So I've refactored it to not require as many
similar macro definitions. I'm going to finish it up and submit it
tomorrow, so please hold on a bit with your additions until then so
that you can base it off the refactored bpf_tracing.h. Thanks.
>
> >>
> >> Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> Sure.
> I'll add the same macro to bpf_tracing.h in selftests.
>
> Thanks!
>
> -----Original Message-----
> From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Sent: Wednesday, December 22, 2021 7:58 AM
> To: Yonghong Song <yhs@fb.com>
> Cc: Tada, Kenta (SGC) <Kenta.Tada@sony.com>; Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
> Subject: Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
>
> On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
> >
> >
> >
> > On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > > But x86_64 Linux System Call convention uses r10 actually.
> > > This commit adds the wrapper for users who want to access to syscall
> > > params to analyze the user space.
> > >
> > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > > ---
> > > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > b/tools/lib/bpf/bpf_tracing.h index db05a5937105..f6fcccd9b10c
> > > 100644
> > > --- a/tools/lib/bpf/bpf_tracing.h
> > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > @@ -67,10 +67,15 @@
> > > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> > >
> > > #define PT_REGS_PARM1(x) ((x)->di)
> > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > #define PT_REGS_PARM2(x) ((x)->si)
> > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > #define PT_REGS_PARM3(x) ((x)->dx)
> > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > #define PT_REGS_PARM4(x) ((x)->cx)
> > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> >
> > I think this is correct. We have a bcc commit doing similar thing.
> > https://urldefense.com/v3/__https://github.com/iovisor/bcc/commit/c234
> > 48e34ecd3cc9bfc19f0b43f4325f77c2e4cc*diff-c78ffb58f59e85eaba9bf9977b72
> > 02f3e50f17e2a9ee556c36a311f9a9ab5d6e__;Iw!!JmoZiZGBv3RvKRSx!qa2pOQlUSd
> > WfoxmZ7EuPvZpWAlK5IBVP1eC-2d3njBLe3yehAXyV_0wI9mGRF5Q$ [github[.]com]
> >
> > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > > #define PT_REGS_RET(x) ((x)->sp)
> > > #define PT_REGS_FP(x) ((x)->bp)
> > > #define PT_REGS_RC(x) ((x)->ax)
> > > @@ -78,10 +83,15 @@
> > > #define PT_REGS_IP(x) ((x)->ip)
> > >
> > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > +syscall uses r10 */
> > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax) @@ -117,10
> > > +127,15 @@
> > > #else
> > >
> > > #define PT_REGS_PARM1(x) ((x)->rdi)
> > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > #define PT_REGS_PARM2(x) ((x)->rsi)
> > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > #define PT_REGS_PARM3(x) ((x)->rdx)
> > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > #define PT_REGS_PARM4(x) ((x)->rcx)
> > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10 */
> > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > > #define PT_REGS_RET(x) ((x)->rsp)
> > > #define PT_REGS_FP(x) ((x)->rbp)
> > > #define PT_REGS_RC(x) ((x)->rax)
> > > @@ -128,10 +143,15 @@
> > > #define PT_REGS_IP(x) ((x)->rip)
> > >
> > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > +syscall uses r10 */
> > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
> >
> > Looks like macros only available for x86_64. Can we make it also
> > available for other architectures so we won't introduce arch specific
> > codes into bpf program?
>
> Yeah, but instead of copy/pasting it for each architecture, let's define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only arch with such inconsistency?) and then after all the architectures defined their macro define
>
> #define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x) ...
> #ifndef PT_REGS_PARM4_SYSCALL(x)
> #define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x) #endif
>
> That way we'll avoid all the extra "no-op" definitions.
>
>
> >
> > Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> +1
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-22 6:47 ` Andrii Nakryiko
@ 2021-12-22 6:57 ` Kenta.Tada
2022-01-06 20:29 ` Andrii Nakryiko
0 siblings, 1 reply; 8+ messages in thread
From: Kenta.Tada @ 2021-12-22 6:57 UTC (permalink / raw)
To: andrii.nakryiko
Cc: yhs, andrii, bpf, ast, daniel, kafai, songliubraving,
john.fastabend, kpsingh
>Let's not add unnecessary #ifndefs. If we get a case for other args, we'll add #ifndefs as necessary.
OK. I agree with you.
>But after looking at your and Hengqi's patches in the last few days, I've looked at the current state of bpf_tracing.h and felt like there is too much repetition. So I've refactored it to not require as many similar macro definitions. I'm going to finish it up and submit it tomorrow, so please hold on a bit with your additions until then so that you can base it off the refactored bpf_tracing.h. Thanks.
I'll wait for the refactored bpf_tracing.h
Thanks for all your help.
-----Original Message-----
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Sent: Wednesday, December 22, 2021 3:47 PM
To: Tada, Kenta (SGC) <Kenta.Tada@sony.com>
Cc: Yonghong Song <yhs@fb.com>; Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
Subject: Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
On Tue, Dec 21, 2021 at 10:20 PM <Kenta.Tada@sony.com> wrote:
>
> >>
> >> Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> I misunderstood this comment.
> I'll just add a test of this new macro.
>
> -----Original Message-----
> From: Tada, Kenta (SGC)
> Sent: Wednesday, December 22, 2021 2:52 PM
> To: Andrii Nakryiko <andrii.nakryiko@gmail.com>; Yonghong Song
> <yhs@fb.com>
> Cc: Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>;
> Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann
> <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu
> <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP
> Singh <kpsingh@kernel.org>
> Subject: RE: [PATCH] libbpf: Fix the incorrect register read for
> syscalls on x86_64
>
> >> Looks like macros only available for x86_64. Can we make it also
> >> available for other architectures so we won't introduce arch
> >> specific codes into bpf program?
> >
> >Yeah, but instead of copy/pasting it for each architecture, let's
> >define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
> >arch with such inconsistency?) and then after all the architectures
> >defined their macro define
>
> Currently, I think x86_64 is the only arch which causes this issue.
> But I'll add #ifndef to not only the 4th parameter but also all parameters for future extensibility.
Let's not add unnecessary #ifndefs. If we get a case for other args, we'll add #ifndefs as necessary.
But after looking at your and Hengqi's patches in the last few days, I've looked at the current state of bpf_tracing.h and felt like there is too much repetition. So I've refactored it to not require as many similar macro definitions. I'm going to finish it up and submit it tomorrow, so please hold on a bit with your additions until then so that you can base it off the refactored bpf_tracing.h. Thanks.
>
> >>
> >> Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> Sure.
> I'll add the same macro to bpf_tracing.h in selftests.
>
> Thanks!
>
> -----Original Message-----
> From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Sent: Wednesday, December 22, 2021 7:58 AM
> To: Yonghong Song <yhs@fb.com>
> Cc: Tada, Kenta (SGC) <Kenta.Tada@sony.com>; Andrii Nakryiko
> <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov
> <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau
> <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend
> <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
> Subject: Re: [PATCH] libbpf: Fix the incorrect register read for
> syscalls on x86_64
>
> On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
> >
> >
> >
> > On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > > But x86_64 Linux System Call convention uses r10 actually.
> > > This commit adds the wrapper for users who want to access to
> > > syscall params to analyze the user space.
> > >
> > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > > ---
> > > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > > 1 file changed, 20 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > b/tools/lib/bpf/bpf_tracing.h index db05a5937105..f6fcccd9b10c
> > > 100644
> > > --- a/tools/lib/bpf/bpf_tracing.h
> > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > @@ -67,10 +67,15 @@
> > > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> > >
> > > #define PT_REGS_PARM1(x) ((x)->di)
> > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > #define PT_REGS_PARM2(x) ((x)->si)
> > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > #define PT_REGS_PARM3(x) ((x)->dx)
> > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > #define PT_REGS_PARM4(x) ((x)->cx)
> > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10
> > > +*/
> >
> > I think this is correct. We have a bcc commit doing similar thing.
> > https://urldefense.com/v3/__https://github.com/iovisor/bcc/commit/c2
> > 34
> > 48e34ecd3cc9bfc19f0b43f4325f77c2e4cc*diff-c78ffb58f59e85eaba9bf9977b
> > 72
> > 02f3e50f17e2a9ee556c36a311f9a9ab5d6e__;Iw!!JmoZiZGBv3RvKRSx!qa2pOQlU
> > Sd WfoxmZ7EuPvZpWAlK5IBVP1eC-2d3njBLe3yehAXyV_0wI9mGRF5Q$
> > [github[.]com]
> >
> > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > > #define PT_REGS_RET(x) ((x)->sp)
> > > #define PT_REGS_FP(x) ((x)->bp)
> > > #define PT_REGS_RC(x) ((x)->ax)
> > > @@ -78,10 +83,15 @@
> > > #define PT_REGS_IP(x) ((x)->ip)
> > >
> > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > +syscall uses r10 */
> > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax) @@ -117,10
> > > +127,15 @@
> > > #else
> > >
> > > #define PT_REGS_PARM1(x) ((x)->rdi)
> > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > #define PT_REGS_PARM2(x) ((x)->rsi)
> > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > #define PT_REGS_PARM3(x) ((x)->rdx)
> > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > #define PT_REGS_PARM4(x) ((x)->rcx)
> > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10
> > > +*/
> > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > > #define PT_REGS_RET(x) ((x)->rsp)
> > > #define PT_REGS_FP(x) ((x)->rbp)
> > > #define PT_REGS_RC(x) ((x)->rax) @@ -128,10 +143,15 @@
> > > #define PT_REGS_IP(x) ((x)->rip)
> > >
> > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > +syscall uses r10 */
> > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
> >
> > Looks like macros only available for x86_64. Can we make it also
> > available for other architectures so we won't introduce arch
> > specific codes into bpf program?
>
> Yeah, but instead of copy/pasting it for each architecture, let's
> define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
> arch with such inconsistency?) and then after all the architectures
> defined their macro define
>
> #define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x) ...
> #ifndef PT_REGS_PARM4_SYSCALL(x)
> #define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x) #endif
>
> That way we'll avoid all the extra "no-op" definitions.
>
>
> >
> > Also, could you add a selftest to use this macro, esp. for parameter 4?
>
> +1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
2021-12-22 6:57 ` Kenta.Tada
@ 2022-01-06 20:29 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2022-01-06 20:29 UTC (permalink / raw)
To: Kenta Tada
Cc: Yonghong Song, Andrii Nakryiko, bpf, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, john fastabend, KP Singh
On Tue, Dec 21, 2021 at 10:58 PM <Kenta.Tada@sony.com> wrote:
>
> >Let's not add unnecessary #ifndefs. If we get a case for other args, we'll add #ifndefs as necessary.
>
> OK. I agree with you.
>
> >But after looking at your and Hengqi's patches in the last few days, I've looked at the current state of bpf_tracing.h and felt like there is too much repetition. So I've refactored it to not require as many similar macro definitions. I'm going to finish it up and submit it tomorrow, so please hold on a bit with your additions until then so that you can base it off the refactored bpf_tracing.h. Thanks.
>
> I'll wait for the refactored bpf_tracing.h
> Thanks for all your help.
The patches got merged. Please submit your fixes based on top of those.
And please don't top post, reply inline.
>
> -----Original Message-----
> From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> Sent: Wednesday, December 22, 2021 3:47 PM
> To: Tada, Kenta (SGC) <Kenta.Tada@sony.com>
> Cc: Yonghong Song <yhs@fb.com>; Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
> Subject: Re: [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64
>
> On Tue, Dec 21, 2021 at 10:20 PM <Kenta.Tada@sony.com> wrote:
> >
> > >>
> > >> Also, could you add a selftest to use this macro, esp. for parameter 4?
> >
> > I misunderstood this comment.
> > I'll just add a test of this new macro.
> >
> > -----Original Message-----
> > From: Tada, Kenta (SGC)
> > Sent: Wednesday, December 22, 2021 2:52 PM
> > To: Andrii Nakryiko <andrii.nakryiko@gmail.com>; Yonghong Song
> > <yhs@fb.com>
> > Cc: Andrii Nakryiko <andrii@kernel.org>; bpf <bpf@vger.kernel.org>;
> > Alexei Starovoitov <ast@kernel.org>; Daniel Borkmann
> > <daniel@iogearbox.net>; Martin Lau <kafai@fb.com>; Song Liu
> > <songliubraving@fb.com>; john fastabend <john.fastabend@gmail.com>; KP
> > Singh <kpsingh@kernel.org>
> > Subject: RE: [PATCH] libbpf: Fix the incorrect register read for
> > syscalls on x86_64
> >
> > >> Looks like macros only available for x86_64. Can we make it also
> > >> available for other architectures so we won't introduce arch
> > >> specific codes into bpf program?
> > >
> > >Yeah, but instead of copy/pasting it for each architecture, let's
> > >define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
> > >arch with such inconsistency?) and then after all the architectures
> > >defined their macro define
> >
> > Currently, I think x86_64 is the only arch which causes this issue.
> > But I'll add #ifndef to not only the 4th parameter but also all parameters for future extensibility.
>
> Let's not add unnecessary #ifndefs. If we get a case for other args, we'll add #ifndefs as necessary.
>
> But after looking at your and Hengqi's patches in the last few days, I've looked at the current state of bpf_tracing.h and felt like there is too much repetition. So I've refactored it to not require as many similar macro definitions. I'm going to finish it up and submit it tomorrow, so please hold on a bit with your additions until then so that you can base it off the refactored bpf_tracing.h. Thanks.
>
> >
> > >>
> > >> Also, could you add a selftest to use this macro, esp. for parameter 4?
> >
> > Sure.
> > I'll add the same macro to bpf_tracing.h in selftests.
> >
> > Thanks!
> >
> > -----Original Message-----
> > From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > Sent: Wednesday, December 22, 2021 7:58 AM
> > To: Yonghong Song <yhs@fb.com>
> > Cc: Tada, Kenta (SGC) <Kenta.Tada@sony.com>; Andrii Nakryiko
> > <andrii@kernel.org>; bpf <bpf@vger.kernel.org>; Alexei Starovoitov
> > <ast@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Martin Lau
> > <kafai@fb.com>; Song Liu <songliubraving@fb.com>; john fastabend
> > <john.fastabend@gmail.com>; KP Singh <kpsingh@kernel.org>
> > Subject: Re: [PATCH] libbpf: Fix the incorrect register read for
> > syscalls on x86_64
> >
> > On Tue, Dec 21, 2021 at 7:51 AM Yonghong Song <yhs@fb.com> wrote:
> > >
> > >
> > >
> > > On 12/21/21 3:21 AM, Kenta.Tada@sony.com wrote:
> > > > Currently, rcx is read as the fourth parameter of syscall on x86_64.
> > > > But x86_64 Linux System Call convention uses r10 actually.
> > > > This commit adds the wrapper for users who want to access to
> > > > syscall params to analyze the user space.
> > > >
> > > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com>
> > > > ---
> > > > tools/lib/bpf/bpf_tracing.h | 20 ++++++++++++++++++++
> > > > 1 file changed, 20 insertions(+)
> > > >
> > > > diff --git a/tools/lib/bpf/bpf_tracing.h
> > > > b/tools/lib/bpf/bpf_tracing.h index db05a5937105..f6fcccd9b10c
> > > > 100644
> > > > --- a/tools/lib/bpf/bpf_tracing.h
> > > > +++ b/tools/lib/bpf/bpf_tracing.h
> > > > @@ -67,10 +67,15 @@
> > > > #if defined(__KERNEL__) || defined(__VMLINUX_H__)
> > > >
> > > > #define PT_REGS_PARM1(x) ((x)->di)
> > > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > > #define PT_REGS_PARM2(x) ((x)->si)
> > > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > > #define PT_REGS_PARM3(x) ((x)->dx)
> > > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > > #define PT_REGS_PARM4(x) ((x)->cx)
> > > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10
> > > > +*/
> > >
> > > I think this is correct. We have a bcc commit doing similar thing.
> > > https://urldefense.com/v3/__https://github.com/iovisor/bcc/commit/c2
> > > 34
> > > 48e34ecd3cc9bfc19f0b43f4325f77c2e4cc*diff-c78ffb58f59e85eaba9bf9977b
> > > 72
> > > 02f3e50f17e2a9ee556c36a311f9a9ab5d6e__;Iw!!JmoZiZGBv3RvKRSx!qa2pOQlU
> > > Sd WfoxmZ7EuPvZpWAlK5IBVP1eC-2d3njBLe3yehAXyV_0wI9mGRF5Q$
> > > [github[.]com]
> > >
> > > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > > +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> > > > #define PT_REGS_RET(x) ((x)->sp)
> > > > #define PT_REGS_FP(x) ((x)->bp)
> > > > #define PT_REGS_RC(x) ((x)->ax)
> > > > @@ -78,10 +83,15 @@
> > > > #define PT_REGS_IP(x) ((x)->ip)
> > > >
> > > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), di)
> > > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), si)
> > > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), dx)
> > > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), cx)
> > > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > > +syscall uses r10 */
> > > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), sp)
> > > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), bp)
> > > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), ax) @@ -117,10
> > > > +127,15 @@
> > > > #else
> > > >
> > > > #define PT_REGS_PARM1(x) ((x)->rdi)
> > > > +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> > > > #define PT_REGS_PARM2(x) ((x)->rsi)
> > > > +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> > > > #define PT_REGS_PARM3(x) ((x)->rdx)
> > > > +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> > > > #define PT_REGS_PARM4(x) ((x)->rcx)
> > > > +#define PT_REGS_PARM4_SYSCALL(x) ((x)->r10) /* syscall uses r10
> > > > +*/
> > > > #define PT_REGS_PARM5(x) ((x)->r8)
> > > > +#define PT_REGS_PARM5(x) PT_REGS_PARM5(x)
> > > > #define PT_REGS_RET(x) ((x)->rsp)
> > > > #define PT_REGS_FP(x) ((x)->rbp)
> > > > #define PT_REGS_RC(x) ((x)->rax) @@ -128,10 +143,15 @@
> > > > #define PT_REGS_IP(x) ((x)->rip)
> > > >
> > > > #define PT_REGS_PARM1_CORE(x) BPF_CORE_READ((x), rdi)
> > > > +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> > > > #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ((x), rsi)
> > > > +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> > > > #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ((x), rdx)
> > > > +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> > > > #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ((x), rcx)
> > > > +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ((x), r10) /*
> > > > +syscall uses r10 */
> > > > #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ((x), r8)
> > > > +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> > > > #define PT_REGS_RET_CORE(x) BPF_CORE_READ((x), rsp)
> > > > #define PT_REGS_FP_CORE(x) BPF_CORE_READ((x), rbp)
> > > > #define PT_REGS_RC_CORE(x) BPF_CORE_READ((x), rax)
> > >
> > > Looks like macros only available for x86_64. Can we make it also
> > > available for other architectures so we won't introduce arch
> > > specific codes into bpf program?
> >
> > Yeah, but instead of copy/pasting it for each architecture, let's
> > define PT_REGS_PARM4/PT_REGS_PARM4_CORE for x86-64 (is this the only
> > arch with such inconsistency?) and then after all the architectures
> > defined their macro define
> >
> > #define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x) ...
> > #ifndef PT_REGS_PARM4_SYSCALL(x)
> > #define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x) #endif
> >
> > That way we'll avoid all the extra "no-op" definitions.
> >
> >
> > >
> > > Also, could you add a selftest to use this macro, esp. for parameter 4?
> >
> > +1
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-06 20:29 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-21 11:21 [PATCH] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta.Tada
2021-12-21 15:50 ` Yonghong Song
2021-12-21 22:58 ` Andrii Nakryiko
2021-12-22 5:51 ` Kenta.Tada
2021-12-22 6:19 ` Kenta.Tada
2021-12-22 6:47 ` Andrii Nakryiko
2021-12-22 6:57 ` Kenta.Tada
2022-01-06 20:29 ` 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.