* [PATCH v5 0/3] Fix the incorrect register read for syscalls on x86_64 @ 2022-01-24 14:16 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 1/3] libbpf: Extract syscall wrapper Kenta Tada ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Kenta Tada @ 2022-01-24 14:16 UTC (permalink / raw) To: andrii, bpf Cc: ast, daniel, kafai, songliubraving, yhs, john.fastabend, kpsingh, Kenta Tada 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/ v2 -> v3: - Modify the definition of SYSCALL macros for only targeted archs. - Define __BPF_TARGET_MISSING variants for completeness. - Remove CORE variants. These macros will not be used. - Add a selftest. v3 -> v4: - Modify a selftest not to use serial tests. - Modify a selftest to use ASSERT_EQ(). - Extract syscall wrapper for all the other tests. - Add CORE variants. v4 -> v5: - Modify the CORE variant macro not to read memory directly. - Remove the unnecessary comment. - Add a selftest for the CORE variant. Kenta Tada (3): libbpf: Extract syscall wrapper libbpf: Fix the incorrect register read for syscalls on x86_64 libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL tools/lib/bpf/bpf_tracing.h | 34 ++++++++++ .../bpf/prog_tests/test_bpf_syscall_macro.c | 63 ++++++++++++++++++ tools/testing/selftests/bpf/progs/bpf_misc.h | 19 ++++++ .../selftests/bpf/progs/bpf_syscall_macro.c | 64 +++++++++++++++++++ .../selftests/bpf/progs/test_probe_user.c | 15 +---- 5 files changed, 181 insertions(+), 14 deletions(-) create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c create mode 100644 tools/testing/selftests/bpf/progs/bpf_misc.h create mode 100644 tools/testing/selftests/bpf/progs/bpf_syscall_macro.c -- 2.32.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/3] libbpf: Extract syscall wrapper 2022-01-24 14:16 [PATCH v5 0/3] Fix the incorrect register read for syscalls on x86_64 Kenta Tada @ 2022-01-24 14:16 ` Kenta Tada 2022-01-24 14:16 ` [PATCH v5 2/3] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL Kenta Tada 2 siblings, 0 replies; 7+ messages in thread From: Kenta Tada @ 2022-01-24 14:16 UTC (permalink / raw) To: andrii, bpf Cc: ast, daniel, kafai, songliubraving, yhs, john.fastabend, kpsingh, Kenta Tada Extract the helper to set up SYS_PREFIX for fentry and kprobe selftests that use __x86_sys_* attach functions. Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> Suggested-by: Andrii Nakryiko <andrii@kernel.org> --- tools/testing/selftests/bpf/progs/bpf_misc.h | 19 +++++++++++++++++++ .../selftests/bpf/progs/test_probe_user.c | 15 +-------------- 2 files changed, 20 insertions(+), 14 deletions(-) create mode 100644 tools/testing/selftests/bpf/progs/bpf_misc.h diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h new file mode 100644 index 000000000000..0b78bc9b1b4c --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __BPF_MISC_H__ +#define __BPF_MISC_H__ + +#if defined(__TARGET_ARCH_x86) +#define SYSCALL_WRAPPER 1 +#define SYS_PREFIX "__x64_" +#elif defined(__TARGET_ARCH_s390) +#define SYSCALL_WRAPPER 1 +#define SYS_PREFIX "__s390x_" +#elif defined(__TARGET_ARCH_arm64) +#define SYSCALL_WRAPPER 1 +#define SYS_PREFIX "__arm64_" +#else +#define SYSCALL_WRAPPER 0 +#define SYS_PREFIX "" +#endif + +#endif diff --git a/tools/testing/selftests/bpf/progs/test_probe_user.c b/tools/testing/selftests/bpf/progs/test_probe_user.c index 8812a90da4eb..702578a5e496 100644 --- a/tools/testing/selftests/bpf/progs/test_probe_user.c +++ b/tools/testing/selftests/bpf/progs/test_probe_user.c @@ -7,20 +7,7 @@ #include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> - -#if defined(__TARGET_ARCH_x86) -#define SYSCALL_WRAPPER 1 -#define SYS_PREFIX "__x64_" -#elif defined(__TARGET_ARCH_s390) -#define SYSCALL_WRAPPER 1 -#define SYS_PREFIX "__s390x_" -#elif defined(__TARGET_ARCH_arm64) -#define SYSCALL_WRAPPER 1 -#define SYS_PREFIX "__arm64_" -#else -#define SYSCALL_WRAPPER 0 -#define SYS_PREFIX "" -#endif +#include "bpf_misc.h" static struct sockaddr_in old; -- 2.32.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/3] libbpf: Fix the incorrect register read for syscalls on x86_64 2022-01-24 14:16 [PATCH v5 0/3] Fix the incorrect register read for syscalls on x86_64 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 1/3] libbpf: Extract syscall wrapper Kenta Tada @ 2022-01-24 14:16 ` Kenta Tada 2022-01-24 14:16 ` [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL Kenta Tada 2 siblings, 0 replies; 7+ messages in thread From: Kenta Tada @ 2022-01-24 14:16 UTC (permalink / raw) To: andrii, bpf Cc: ast, daniel, kafai, songliubraving, yhs, john.fastabend, kpsingh, Kenta Tada 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 | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h index 90f56b0f585f..032ba809f3e5 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 @@ -263,6 +265,26 @@ struct pt_regs; #endif +#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) +#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 +#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) +#ifdef __PT_PARM4_REG_SYSCALL +#define PT_REGS_PARM4_CORE_SYSCALL(x) BPF_CORE_READ(__PT_REGS_CAST(x), __PT_PARM4_REG_SYSCALL) +#else /* __PT_PARM4_REG_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) + #else /* defined(bpf_target_defined) */ #define PT_REGS_PARM1(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) @@ -290,6 +312,18 @@ struct pt_regs; #define BPF_KPROBE_READ_RET_IP(ip, ctx) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) #define BPF_KRETPROBE_READ_RET_IP(ip, ctx) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM1_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM2_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM3_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM4_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM5_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) + +#define PT_REGS_PARM1_CORE_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM2_CORE_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM3_CORE_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM4_CORE_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) +#define PT_REGS_PARM5_CORE_SYSCALL(x) ({ _Pragma(__BPF_TARGET_MISSING); 0l; }) + #endif /* defined(bpf_target_defined) */ #ifndef ___bpf_concat -- 2.32.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL 2022-01-24 14:16 [PATCH v5 0/3] Fix the incorrect register read for syscalls on x86_64 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 1/3] libbpf: Extract syscall wrapper Kenta Tada 2022-01-24 14:16 ` [PATCH v5 2/3] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta Tada @ 2022-01-24 14:16 ` Kenta Tada 2022-01-25 5:05 ` Andrii Nakryiko 2 siblings, 1 reply; 7+ messages in thread From: Kenta Tada @ 2022-01-24 14:16 UTC (permalink / raw) To: andrii, bpf Cc: ast, daniel, kafai, songliubraving, yhs, john.fastabend, kpsingh, Kenta Tada Add a selftest to verify the behavior of PT_REGS_xxx and the CORE variant. Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> --- .../bpf/prog_tests/test_bpf_syscall_macro.c | 63 ++++++++++++++++++ .../selftests/bpf/progs/bpf_syscall_macro.c | 64 +++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c create mode 100644 tools/testing/selftests/bpf/progs/bpf_syscall_macro.c diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c new file mode 100644 index 000000000000..f5f4c8adf539 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright 2022 Sony Group Corporation */ +#include <sys/prctl.h> +#include <test_progs.h> +#include "bpf_syscall_macro.skel.h" + +void test_bpf_syscall_macro(void) +{ + struct bpf_syscall_macro *skel = NULL; + int err; + int exp_arg1 = 1001; + unsigned long exp_arg2 = 12; + unsigned long exp_arg3 = 13; + unsigned long exp_arg4 = 14; + unsigned long exp_arg5 = 15; + + /* check whether it can open program */ + skel = bpf_syscall_macro__open(); + if (!ASSERT_OK_PTR(skel, "bpf_syscall_macro__open")) + return; + + skel->rodata->filter_pid = getpid(); + + /* check whether it can load program */ + err = bpf_syscall_macro__load(skel); + if (!ASSERT_OK(err, "bpf_syscall_macro__load")) + goto cleanup; + + /* check whether it can attach kprobe */ + err = bpf_syscall_macro__attach(skel); + if (!ASSERT_OK(err, "bpf_syscall_macro__attach")) + goto cleanup; + + /* check whether args of syscall are copied correctly */ + prctl(exp_arg1, exp_arg2, exp_arg3, exp_arg4, exp_arg5); + ASSERT_EQ(skel->bss->arg1, exp_arg1, "syscall_arg1"); + ASSERT_EQ(skel->bss->arg2, exp_arg2, "syscall_arg2"); + ASSERT_EQ(skel->bss->arg3, exp_arg3, "syscall_arg3"); + /* it cannot copy arg4 when uses PT_REGS_PARM4 on x86_64 */ +#ifdef __x86_64__ + ASSERT_NEQ(skel->bss->arg4_cx, exp_arg4, "syscall_arg4_from_cx"); +#else + ASSERT_EQ(skel->bss->arg4_cx, exp_arg4, "syscall_arg4_from_cx"); +#endif + ASSERT_EQ(skel->bss->arg4, exp_arg4, "syscall_arg4"); + ASSERT_EQ(skel->bss->arg5, exp_arg5, "syscall_arg5"); + + /* check whether args of syscall are copied correctly for CORE variants */ + ASSERT_EQ(skel->bss->arg1_core, exp_arg1, "syscall_arg1_core_variant"); + ASSERT_EQ(skel->bss->arg2_core, exp_arg2, "syscall_arg2_core_variant"); + ASSERT_EQ(skel->bss->arg3_core, exp_arg3, "syscall_arg3_core_variant"); + /* it cannot copy arg4 when uses PT_REGS_PARM4_CORE on x86_64 */ +#ifdef __x86_64__ + ASSERT_NEQ(skel->bss->arg4_core_cx, exp_arg4, "syscall_arg4_from_cx_core_variant"); +#else + ASSERT_EQ(skel->bss->arg4_core_cx, exp_arg4, "syscall_arg4_from_cx_core_variant"); +#endif + ASSERT_EQ(skel->bss->arg4_core, exp_arg4, "syscall_arg4_core_variant"); + ASSERT_EQ(skel->bss->arg5_core, exp_arg5, "syscall_arg5_core_variant"); + +cleanup: + bpf_syscall_macro__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c new file mode 100644 index 000000000000..cfeccd85f40e --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c @@ -0,0 +1,64 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright 2022 Sony Group Corporation */ +#include <vmlinux.h> + +#include <bpf/bpf_core_read.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include "bpf_misc.h" + +int arg1 = 0; +unsigned long arg2 = 0; +unsigned long arg3 = 0; +unsigned long arg4_cx = 0; +unsigned long arg4 = 0; +unsigned long arg5 = 0; + +int arg1_core = 0; +unsigned long arg2_core = 0; +unsigned long arg3_core = 0; +unsigned long arg4_core_cx = 0; +unsigned long arg4_core = 0; +unsigned long arg5_core = 0; + +const volatile pid_t filter_pid = 0; + +SEC("kprobe/" SYS_PREFIX "sys_prctl") +int BPF_KPROBE(handle_sys_prctl) +{ + struct pt_regs *real_regs; + int orig_arg1; + unsigned long orig_arg2, orig_arg3, orig_arg4_cx, orig_arg4, orig_arg5; + pid_t pid = bpf_get_current_pid_tgid() >> 32; + + if (pid != filter_pid) + return 0; + + /* test for PT_REGS_PARM */ + real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx); + bpf_probe_read_kernel(&orig_arg1, sizeof(orig_arg1), &PT_REGS_PARM1_SYSCALL(real_regs)); + bpf_probe_read_kernel(&orig_arg2, sizeof(orig_arg2), &PT_REGS_PARM2_SYSCALL(real_regs)); + bpf_probe_read_kernel(&orig_arg3, sizeof(orig_arg3), &PT_REGS_PARM3_SYSCALL(real_regs)); + bpf_probe_read_kernel(&orig_arg4_cx, sizeof(orig_arg4_cx), &PT_REGS_PARM4(real_regs)); + bpf_probe_read_kernel(&orig_arg4, sizeof(orig_arg4), &PT_REGS_PARM4_SYSCALL(real_regs)); + bpf_probe_read_kernel(&orig_arg5, sizeof(orig_arg5), &PT_REGS_PARM5_SYSCALL(real_regs)); + /* copy all actual args and the wrong arg4 on x86_64 */ + arg1 = orig_arg1; + arg2 = orig_arg2; + arg3 = orig_arg3; + arg4_cx = orig_arg4_cx; + arg4 = orig_arg4; + arg5 = orig_arg5; + + /* test for the CORE variant of PT_REGS_PARM */ + arg1_core = PT_REGS_PARM1_CORE_SYSCALL(real_regs); + arg2_core = PT_REGS_PARM2_CORE_SYSCALL(real_regs); + arg3_core = PT_REGS_PARM3_CORE_SYSCALL(real_regs); + arg4_core_cx = PT_REGS_PARM4_CORE(real_regs); + arg4_core = PT_REGS_PARM4_CORE_SYSCALL(real_regs); + arg5_core = PT_REGS_PARM5_CORE_SYSCALL(real_regs); + + return 0; +} + +char _license[] SEC("license") = "GPL"; -- 2.32.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL 2022-01-24 14:16 ` [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL Kenta Tada @ 2022-01-25 5:05 ` Andrii Nakryiko 2022-02-01 19:36 ` Andrii Nakryiko 0 siblings, 1 reply; 7+ messages in thread From: Andrii Nakryiko @ 2022-01-25 5:05 UTC (permalink / raw) To: Kenta Tada, Hengqi Chen Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song, john fastabend, KP Singh +cc Hengqi On Mon, Jan 24, 2022 at 6:20 AM Kenta Tada <Kenta.Tada@sony.com> wrote: > > Add a selftest to verify the behavior of PT_REGS_xxx > and the CORE variant. > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> > --- > .../bpf/prog_tests/test_bpf_syscall_macro.c | 63 ++++++++++++++++++ > .../selftests/bpf/progs/bpf_syscall_macro.c | 64 +++++++++++++++++++ > 2 files changed, 127 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c > create mode 100644 tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > [...] > diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > new file mode 100644 > index 000000000000..cfeccd85f40e > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > @@ -0,0 +1,64 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright 2022 Sony Group Corporation */ > +#include <vmlinux.h> > + > +#include <bpf/bpf_core_read.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include "bpf_misc.h" > + > +int arg1 = 0; > +unsigned long arg2 = 0; > +unsigned long arg3 = 0; > +unsigned long arg4_cx = 0; > +unsigned long arg4 = 0; > +unsigned long arg5 = 0; > + > +int arg1_core = 0; > +unsigned long arg2_core = 0; > +unsigned long arg3_core = 0; > +unsigned long arg4_core_cx = 0; > +unsigned long arg4_core = 0; > +unsigned long arg5_core = 0; > + > +const volatile pid_t filter_pid = 0; > + > +SEC("kprobe/" SYS_PREFIX "sys_prctl") > +int BPF_KPROBE(handle_sys_prctl) > +{ > + struct pt_regs *real_regs; > + int orig_arg1; > + unsigned long orig_arg2, orig_arg3, orig_arg4_cx, orig_arg4, orig_arg5; > + pid_t pid = bpf_get_current_pid_tgid() >> 32; > + > + if (pid != filter_pid) > + return 0; > + > + /* test for PT_REGS_PARM */ > + real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx); > + bpf_probe_read_kernel(&orig_arg1, sizeof(orig_arg1), &PT_REGS_PARM1_SYSCALL(real_regs)); > + bpf_probe_read_kernel(&orig_arg2, sizeof(orig_arg2), &PT_REGS_PARM2_SYSCALL(real_regs)); > + bpf_probe_read_kernel(&orig_arg3, sizeof(orig_arg3), &PT_REGS_PARM3_SYSCALL(real_regs)); > + bpf_probe_read_kernel(&orig_arg4_cx, sizeof(orig_arg4_cx), &PT_REGS_PARM4(real_regs)); > + bpf_probe_read_kernel(&orig_arg4, sizeof(orig_arg4), &PT_REGS_PARM4_SYSCALL(real_regs)); > + bpf_probe_read_kernel(&orig_arg5, sizeof(orig_arg5), &PT_REGS_PARM5_SYSCALL(real_regs)); > + /* copy all actual args and the wrong arg4 on x86_64 */ > + arg1 = orig_arg1; > + arg2 = orig_arg2; > + arg3 = orig_arg3; > + arg4_cx = orig_arg4_cx; > + arg4 = orig_arg4; > + arg5 = orig_arg5; I don't get why you needed orig_argX variables and then copying them into argX variables. I changed this to read directly into argX. I suspect arg1 handling might break on big-endian arches due to int vs long differences, please check that and send a follow up fix. Also keep in mind that selftest changes should come with "selftests/bpf:" subject prefix, not "libbpf:". Fixed that up as well. Applied to bpf-next, thanks. Hopefully Hengqi will build his SYSCALL prog wrapper macros on top of this as a follow up as well. > + > + /* test for the CORE variant of PT_REGS_PARM */ > + arg1_core = PT_REGS_PARM1_CORE_SYSCALL(real_regs); > + arg2_core = PT_REGS_PARM2_CORE_SYSCALL(real_regs); > + arg3_core = PT_REGS_PARM3_CORE_SYSCALL(real_regs); > + arg4_core_cx = PT_REGS_PARM4_CORE(real_regs); > + arg4_core = PT_REGS_PARM4_CORE_SYSCALL(real_regs); > + arg5_core = PT_REGS_PARM5_CORE_SYSCALL(real_regs); > + > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.32.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL 2022-01-25 5:05 ` Andrii Nakryiko @ 2022-02-01 19:36 ` Andrii Nakryiko 2022-02-01 21:01 ` Ilya Leoshkevich 0 siblings, 1 reply; 7+ messages in thread From: Andrii Nakryiko @ 2022-02-01 19:36 UTC (permalink / raw) To: Kenta Tada, Hengqi Chen, Ilya Leoshkevich Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song, john fastabend, KP Singh +cc Ilya On Mon, Jan 24, 2022 at 9:05 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > +cc Hengqi > > On Mon, Jan 24, 2022 at 6:20 AM Kenta Tada <Kenta.Tada@sony.com> wrote: > > > > Add a selftest to verify the behavior of PT_REGS_xxx > > and the CORE variant. > > > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> > > --- > > .../bpf/prog_tests/test_bpf_syscall_macro.c | 63 ++++++++++++++++++ > > .../selftests/bpf/progs/bpf_syscall_macro.c | 64 +++++++++++++++++++ > > 2 files changed, 127 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c > > create mode 100644 tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > > > [...] > > > diff --git a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > new file mode 100644 > > index 000000000000..cfeccd85f40e > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > @@ -0,0 +1,64 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* Copyright 2022 Sony Group Corporation */ > > +#include <vmlinux.h> > > + > > +#include <bpf/bpf_core_read.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > +#include "bpf_misc.h" > > + > > +int arg1 = 0; > > +unsigned long arg2 = 0; > > +unsigned long arg3 = 0; > > +unsigned long arg4_cx = 0; > > +unsigned long arg4 = 0; > > +unsigned long arg5 = 0; > > + > > +int arg1_core = 0; > > +unsigned long arg2_core = 0; > > +unsigned long arg3_core = 0; > > +unsigned long arg4_core_cx = 0; > > +unsigned long arg4_core = 0; > > +unsigned long arg5_core = 0; > > + > > +const volatile pid_t filter_pid = 0; > > + > > +SEC("kprobe/" SYS_PREFIX "sys_prctl") > > +int BPF_KPROBE(handle_sys_prctl) > > +{ > > + struct pt_regs *real_regs; > > + int orig_arg1; > > + unsigned long orig_arg2, orig_arg3, orig_arg4_cx, orig_arg4, orig_arg5; > > + pid_t pid = bpf_get_current_pid_tgid() >> 32; > > + > > + if (pid != filter_pid) > > + return 0; > > + > > + /* test for PT_REGS_PARM */ > > + real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx); > > + bpf_probe_read_kernel(&orig_arg1, sizeof(orig_arg1), &PT_REGS_PARM1_SYSCALL(real_regs)); > > + bpf_probe_read_kernel(&orig_arg2, sizeof(orig_arg2), &PT_REGS_PARM2_SYSCALL(real_regs)); > > + bpf_probe_read_kernel(&orig_arg3, sizeof(orig_arg3), &PT_REGS_PARM3_SYSCALL(real_regs)); > > + bpf_probe_read_kernel(&orig_arg4_cx, sizeof(orig_arg4_cx), &PT_REGS_PARM4(real_regs)); > > + bpf_probe_read_kernel(&orig_arg4, sizeof(orig_arg4), &PT_REGS_PARM4_SYSCALL(real_regs)); > > + bpf_probe_read_kernel(&orig_arg5, sizeof(orig_arg5), &PT_REGS_PARM5_SYSCALL(real_regs)); > > + /* copy all actual args and the wrong arg4 on x86_64 */ > > + arg1 = orig_arg1; > > + arg2 = orig_arg2; > > + arg3 = orig_arg3; > > + arg4_cx = orig_arg4_cx; > > + arg4 = orig_arg4; > > + arg5 = orig_arg5; > > I don't get why you needed orig_argX variables and then copying them > into argX variables. I changed this to read directly into argX. I > suspect arg1 handling might break on big-endian arches due to int vs > long differences, please check that and send a follow up fix. > > Also keep in mind that selftest changes should come with > "selftests/bpf:" subject prefix, not "libbpf:". Fixed that up as well. > > Applied to bpf-next, thanks. This selftest is failing on s390x (see [0]). Ilya, do you know if something special needs to be done for s390x for this case? Here are the two failures: test_bpf_syscall_macro:FAIL:syscall_arg1 unexpected syscall_arg1: actual -1 != expected 1001 test_bpf_syscall_macro:FAIL:syscall_arg1_core_variant unexpected syscall_arg1_core_variant: actual -38 != expected 1001 [0] https://github.com/libbpf/libbpf/runs/5025905587?check_suite_focus=true > > Hopefully Hengqi will build his SYSCALL prog wrapper macros on top of > this as a follow up as well. > > > + > > + /* test for the CORE variant of PT_REGS_PARM */ > > + arg1_core = PT_REGS_PARM1_CORE_SYSCALL(real_regs); > > + arg2_core = PT_REGS_PARM2_CORE_SYSCALL(real_regs); > > + arg3_core = PT_REGS_PARM3_CORE_SYSCALL(real_regs); > > + arg4_core_cx = PT_REGS_PARM4_CORE(real_regs); > > + arg4_core = PT_REGS_PARM4_CORE_SYSCALL(real_regs); > > + arg5_core = PT_REGS_PARM5_CORE_SYSCALL(real_regs); > > + > > + return 0; > > +} > > + > > +char _license[] SEC("license") = "GPL"; > > -- > > 2.32.0 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL 2022-02-01 19:36 ` Andrii Nakryiko @ 2022-02-01 21:01 ` Ilya Leoshkevich 0 siblings, 0 replies; 7+ messages in thread From: Ilya Leoshkevich @ 2022-02-01 21:01 UTC (permalink / raw) To: Andrii Nakryiko, Kenta Tada, Hengqi Chen Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song, john fastabend, KP Singh On Tue, 2022-02-01 at 11:36 -0800, Andrii Nakryiko wrote: > +cc Ilya > > > On Mon, Jan 24, 2022 at 9:05 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > +cc Hengqi > > > > On Mon, Jan 24, 2022 at 6:20 AM Kenta Tada <Kenta.Tada@sony.com> > > wrote: > > > > > > Add a selftest to verify the behavior of PT_REGS_xxx > > > and the CORE variant. > > > > > > Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> > > > --- > > > .../bpf/prog_tests/test_bpf_syscall_macro.c | 63 > > > ++++++++++++++++++ > > > .../selftests/bpf/progs/bpf_syscall_macro.c | 64 > > > +++++++++++++++++++ > > > 2 files changed, 127 insertions(+) > > > create mode 100644 > > > tools/testing/selftests/bpf/prog_tests/test_bpf_syscall_macro.c > > > create mode 100644 > > > tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > > > > > > [...] > > > > > diff --git > > > a/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > > b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > > new file mode 100644 > > > index 000000000000..cfeccd85f40e > > > --- /dev/null > > > +++ b/tools/testing/selftests/bpf/progs/bpf_syscall_macro.c > > > @@ -0,0 +1,64 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* Copyright 2022 Sony Group Corporation */ > > > +#include <vmlinux.h> > > > + > > > +#include <bpf/bpf_core_read.h> > > > +#include <bpf/bpf_helpers.h> > > > +#include <bpf/bpf_tracing.h> > > > +#include "bpf_misc.h" > > > + > > > +int arg1 = 0; > > > +unsigned long arg2 = 0; > > > +unsigned long arg3 = 0; > > > +unsigned long arg4_cx = 0; > > > +unsigned long arg4 = 0; > > > +unsigned long arg5 = 0; > > > + > > > +int arg1_core = 0; > > > +unsigned long arg2_core = 0; > > > +unsigned long arg3_core = 0; > > > +unsigned long arg4_core_cx = 0; > > > +unsigned long arg4_core = 0; > > > +unsigned long arg5_core = 0; > > > + > > > +const volatile pid_t filter_pid = 0; > > > + > > > +SEC("kprobe/" SYS_PREFIX "sys_prctl") > > > +int BPF_KPROBE(handle_sys_prctl) > > > +{ > > > + struct pt_regs *real_regs; > > > + int orig_arg1; > > > + unsigned long orig_arg2, orig_arg3, orig_arg4_cx, > > > orig_arg4, orig_arg5; > > > + pid_t pid = bpf_get_current_pid_tgid() >> 32; > > > + > > > + if (pid != filter_pid) > > > + return 0; > > > + > > > + /* test for PT_REGS_PARM */ > > > + real_regs = (struct pt_regs *)PT_REGS_PARM1(ctx); > > > + bpf_probe_read_kernel(&orig_arg1, sizeof(orig_arg1), > > > &PT_REGS_PARM1_SYSCALL(real_regs)); > > > + bpf_probe_read_kernel(&orig_arg2, sizeof(orig_arg2), > > > &PT_REGS_PARM2_SYSCALL(real_regs)); > > > + bpf_probe_read_kernel(&orig_arg3, sizeof(orig_arg3), > > > &PT_REGS_PARM3_SYSCALL(real_regs)); > > > + bpf_probe_read_kernel(&orig_arg4_cx, > > > sizeof(orig_arg4_cx), &PT_REGS_PARM4(real_regs)); > > > + bpf_probe_read_kernel(&orig_arg4, sizeof(orig_arg4), > > > &PT_REGS_PARM4_SYSCALL(real_regs)); > > > + bpf_probe_read_kernel(&orig_arg5, sizeof(orig_arg5), > > > &PT_REGS_PARM5_SYSCALL(real_regs)); > > > + /* copy all actual args and the wrong arg4 on x86_64 */ > > > + arg1 = orig_arg1; > > > + arg2 = orig_arg2; > > > + arg3 = orig_arg3; > > > + arg4_cx = orig_arg4_cx; > > > + arg4 = orig_arg4; > > > + arg5 = orig_arg5; > > > > I don't get why you needed orig_argX variables and then copying > > them > > into argX variables. I changed this to read directly into argX. I > > suspect arg1 handling might break on big-endian arches due to int > > vs > > long differences, please check that and send a follow up fix. > > > > Also keep in mind that selftest changes should come with > > "selftests/bpf:" subject prefix, not "libbpf:". Fixed that up as > > well. > > > > Applied to bpf-next, thanks. > > This selftest is failing on s390x (see [0]). Ilya, do you know if > something special needs to be done for s390x for this case? > > Here are the two failures: > > test_bpf_syscall_macro:FAIL:syscall_arg1 unexpected syscall_arg1: > actual -1 != expected 1001 > test_bpf_syscall_macro:FAIL:syscall_arg1_core_variant unexpected > syscall_arg1_core_variant: actual -38 != expected 1001 > > [0] > https://github.com/libbpf/libbpf/runs/5025905587?check_suite_focus=true > I think we need to use orig_gpr2 for the first syscall argument (see arch/s390/include/asm/syscall.h). I'll test it and send a patch. > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-01 21:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-24 14:16 [PATCH v5 0/3] Fix the incorrect register read for syscalls on x86_64 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 1/3] libbpf: Extract syscall wrapper Kenta Tada 2022-01-24 14:16 ` [PATCH v5 2/3] libbpf: Fix the incorrect register read for syscalls on x86_64 Kenta Tada 2022-01-24 14:16 ` [PATCH v5 3/3] libbpf: Add a test to confirm PT_REGS_PARM4_SYSCALL Kenta Tada 2022-01-25 5:05 ` Andrii Nakryiko 2022-02-01 19:36 ` Andrii Nakryiko 2022-02-01 21:01 ` Ilya Leoshkevich
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.