* [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64
@ 2022-01-07 2:38 Kenta.Tada
2022-01-07 16:11 ` Yonghong Song
2022-01-07 20:34 ` Andrii Nakryiko
0 siblings, 2 replies; 4+ messages in thread
From: Kenta.Tada @ 2022-01-07 2:38 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.
Changelog:
----------
v1 -> v2:
- Rebase to current bpf-next
https://lore.kernel.org/bpf/20211222213924.1869758-1-andrii@kernel.org/
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 90f56b0f585f..4c3df8c122a4 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -70,6 +70,7 @@
#define __PT_PARM2_REG si
#define __PT_PARM3_REG dx
#define __PT_PARM4_REG cx
+#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
#define __PT_PARM5_REG r8
#define __PT_RET_REG sp
#define __PT_FP_REG bp
@@ -99,6 +100,7 @@
#define __PT_PARM2_REG rsi
#define __PT_PARM3_REG rdx
#define __PT_PARM4_REG rcx
+#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
#define __PT_PARM5_REG r8
#define __PT_RET_REG rsp
#define __PT_FP_REG rbp
@@ -226,6 +228,7 @@ struct pt_regs;
#define PT_REGS_PARM2(x) (__PT_REGS_CAST(x)->__PT_PARM2_REG)
#define PT_REGS_PARM3(x) (__PT_REGS_CAST(x)->__PT_PARM3_REG)
#define PT_REGS_PARM4(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG)
+#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG_SYSCALL)
#define PT_REGS_PARM5(x) (__PT_REGS_CAST(x)->__PT_PARM5_REG)
#define PT_REGS_RET(x) (__PT_REGS_CAST(x)->__PT_RET_REG)
#define PT_REGS_FP(x) (__PT_REGS_CAST(x)->__PT_FP_REG)
@@ -237,6 +240,7 @@ struct pt_regs;
#define PT_REGS_PARM2_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM2_REG)
#define PT_REGS_PARM3_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM3_REG)
#define PT_REGS_PARM4_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG)
+#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL)
#define PT_REGS_PARM5_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM5_REG)
#define PT_REGS_RET_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_RET_REG)
#define PT_REGS_FP_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_FP_REG)
@@ -292,6 +296,22 @@ struct pt_regs;
#endif /* defined(bpf_target_defined) */
+#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
+#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
+#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
+#ifndef PT_REGS_PARM4_SYSCALL
+#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
+#endif
+#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
+
+#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
+#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
+#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
+#ifndef PT_REGS_PARM4_CORE_SYSCALL
+#define PT_REGS_PARM4_CORE_SYSCALL(x) PT_REGS_PARM4_CORE(x)
+#endif
+#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
+
#ifndef ___bpf_concat
#define ___bpf_concat(a, b) a ## b
#endif
--
2.32.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64
2022-01-07 2:38 [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta.Tada
@ 2022-01-07 16:11 ` Yonghong Song
2022-01-07 20:34 ` Andrii Nakryiko
1 sibling, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2022-01-07 16:11 UTC (permalink / raw)
To: Kenta.Tada, andrii, bpf
Cc: ast, daniel, kafai, songliubraving, john.fastabend, kpsingh
On 1/6/22 6:38 PM, 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.
>
> Changelog:
> ----------
> v1 -> v2:
> - Rebase to current bpf-next
> https://lore.kernel.org/bpf/20211222213924.1869758-1-andrii@kernel.org/
>
> 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 90f56b0f585f..4c3df8c122a4 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -70,6 +70,7 @@
> #define __PT_PARM2_REG si
> #define __PT_PARM3_REG dx
> #define __PT_PARM4_REG cx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
> #define __PT_PARM5_REG r8
> #define __PT_RET_REG sp
> #define __PT_FP_REG bp
> @@ -99,6 +100,7 @@
> #define __PT_PARM2_REG rsi
> #define __PT_PARM3_REG rdx
> #define __PT_PARM4_REG rcx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
> #define __PT_PARM5_REG r8
> #define __PT_RET_REG rsp
> #define __PT_FP_REG rbp
> @@ -226,6 +228,7 @@ struct pt_regs;
> #define PT_REGS_PARM2(x) (__PT_REGS_CAST(x)->__PT_PARM2_REG)
> #define PT_REGS_PARM3(x) (__PT_REGS_CAST(x)->__PT_PARM3_REG)
> #define PT_REGS_PARM4(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG)
> +#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG_SYSCALL)
> #define PT_REGS_PARM5(x) (__PT_REGS_CAST(x)->__PT_PARM5_REG)
> #define PT_REGS_RET(x) (__PT_REGS_CAST(x)->__PT_RET_REG)
> #define PT_REGS_FP(x) (__PT_REGS_CAST(x)->__PT_FP_REG)
> @@ -237,6 +240,7 @@ struct pt_regs;
> #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM2_REG)
> #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM3_REG)
> #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG)
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL)
> #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM5_REG)
> #define PT_REGS_RET_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_RET_REG)
> #define PT_REGS_FP_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_FP_REG)
> @@ -292,6 +296,22 @@ struct pt_regs;
>
> #endif /* defined(bpf_target_defined) */
>
> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> +#ifndef PT_REGS_PARM4_SYSCALL
> +#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
> +#endif
> +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> +
> +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> +#ifndef PT_REGS_PARM4_CORE_SYSCALL
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) PT_REGS_PARM4_CORE(x)
> +#endif
> +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
Since you proposed the patch, you probably hit the issue. Please
add a selftest so the macro esp. PT_REGS_PARM4_SYSCALL or
PT_REGS_PARM4_CORE_SYSCALL is exercised. Thanks.
> +
> #ifndef ___bpf_concat
> #define ___bpf_concat(a, b) a ## b
> #endif
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64
2022-01-07 2:38 [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta.Tada
2022-01-07 16:11 ` Yonghong Song
@ 2022-01-07 20:34 ` Andrii Nakryiko
2022-01-11 1:04 ` Kenta.Tada
1 sibling, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2022-01-07 20:34 UTC (permalink / raw)
To: Kenta Tada
Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann,
Martin Lau, Song Liu, Yonghong Song, john fastabend, KP Singh
On Thu, Jan 6, 2022 at 6:39 PM <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.
>
> Changelog:
> ----------
> v1 -> v2:
> - Rebase to current bpf-next
> https://lore.kernel.org/bpf/20211222213924.1869758-1-andrii@kernel.org/
>
> 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 90f56b0f585f..4c3df8c122a4 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -70,6 +70,7 @@
> #define __PT_PARM2_REG si
> #define __PT_PARM3_REG dx
> #define __PT_PARM4_REG cx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
> #define __PT_PARM5_REG r8
> #define __PT_RET_REG sp
> #define __PT_FP_REG bp
> @@ -99,6 +100,7 @@
> #define __PT_PARM2_REG rsi
> #define __PT_PARM3_REG rdx
> #define __PT_PARM4_REG rcx
> +#define __PT_PARM4_REG_SYSCALL r10 /* syscall uses r10 */
> #define __PT_PARM5_REG r8
> #define __PT_RET_REG rsp
> #define __PT_FP_REG rbp
> @@ -226,6 +228,7 @@ struct pt_regs;
> #define PT_REGS_PARM2(x) (__PT_REGS_CAST(x)->__PT_PARM2_REG)
> #define PT_REGS_PARM3(x) (__PT_REGS_CAST(x)->__PT_PARM3_REG)
> #define PT_REGS_PARM4(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG)
> +#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG_SYSCALL)
Not all architectures define __PT_PARM4_REG_SYSCALL, but you are
assuming it does. I think you mixed up where to put these definitions,
see below.
> #define PT_REGS_PARM5(x) (__PT_REGS_CAST(x)->__PT_PARM5_REG)
> #define PT_REGS_RET(x) (__PT_REGS_CAST(x)->__PT_RET_REG)
> #define PT_REGS_FP(x) (__PT_REGS_CAST(x)->__PT_FP_REG)
> @@ -237,6 +240,7 @@ struct pt_regs;
> #define PT_REGS_PARM2_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM2_REG)
> #define PT_REGS_PARM3_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM3_REG)
> #define PT_REGS_PARM4_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG)
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL)
Do we need CORE variants for _SYSCALL macro? I guess realistic
selftests would show that.
> #define PT_REGS_PARM5_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM5_REG)
> #define PT_REGS_RET_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_RET_REG)
> #define PT_REGS_FP_CORE(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_FP_REG)
> @@ -292,6 +296,22 @@ struct pt_regs;
>
> #endif /* defined(bpf_target_defined) */
>
> +#define PT_REGS_PARM1_SYSCALL(x) PT_REGS_PARM1(x)
> +#define PT_REGS_PARM2_SYSCALL(x) PT_REGS_PARM2(x)
> +#define PT_REGS_PARM3_SYSCALL(x) PT_REGS_PARM3(x)
> +#ifndef PT_REGS_PARM4_SYSCALL
> +#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
> +#endif
> +#define PT_REGS_PARM5_SYSCALL(x) PT_REGS_PARM5(x)
> +
> +#define PT_REGS_PARM1_CORE_SYSCALL(x) PT_REGS_PARM1_CORE(x)
> +#define PT_REGS_PARM2_CORE_SYSCALL(x) PT_REGS_PARM2_CORE(x)
> +#define PT_REGS_PARM3_CORE_SYSCALL(x) PT_REGS_PARM3_CORE(x)
> +#ifndef PT_REGS_PARM4_CORE_SYSCALL
> +#define PT_REGS_PARM4_CORE_SYSCALL(x) PT_REGS_PARM4_CORE(x)
> +#endif
> +#define PT_REGS_PARM5_CORE_SYSCALL(x) PT_REGS_PARM5_CORE(x)
> +
All this should be inside the `#if defined(bpf_target_defined)`
section, not after it. And then for PT_REGS_PARM4_SYSCALL you can just
do this:
#ifdef __PT_PARM4_REG_SYSCALL
#define PT_REGS_PARM4_SYSCALL(x) (__PT_REGS_CAST(x)->__PT_PARM4_REG_SYSCALL)
#else /* __PT_PARM4_REG_SYSCALL */
#define PT_REGS_PARM4_SYSCALL(x) PT_REGS_PARM4(x)
#endif
And I guess for completeness we need to define __BPF_TARGET_MISSING
variants if !defined(bpf_traget_defined), see how it's done for all
current PT_REGS_xxx
> #ifndef ___bpf_concat
> #define ___bpf_concat(a, b) a ## b
> #endif
> --
> 2.32.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64
2022-01-07 20:34 ` Andrii Nakryiko
@ 2022-01-11 1:04 ` Kenta.Tada
0 siblings, 0 replies; 4+ messages in thread
From: Kenta.Tada @ 2022-01-11 1:04 UTC (permalink / raw)
To: andrii.nakryiko
Cc: andrii, bpf, ast, daniel, kafai, songliubraving, yhs,
john.fastabend, kpsingh
I'll reconsider whether we need CORE variants when I create selftests and fix the problem you pointed out.
I'm sorry but my response might be a little late due to my personal reason.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-11 1:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-07 2:38 [PATCH v2] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta.Tada
2022-01-07 16:11 ` Yonghong Song
2022-01-07 20:34 ` Andrii Nakryiko
2022-01-11 1:04 ` Kenta.Tada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).