[PATCH bpf-next v2 0/2] libbpf: Add syscall-specific variant of BPF_KPROBE

[PATCH bpf-next v2 0/2] libbpf: Add syscall-specific variant of BPF_KPROBE

[Hengqi Chen]

Add new macro BPF_KPROBE_SYSCALL, which provides easy access to syscall
input arguments. See [0] and [1] for background.

  [0]: https://github.com/libbpf/libbpf-bootstrap/issues/57
  [1]: https://github.com/libbpf/libbpf/issues/425

Hengqi Chen (2):
  libbpf: Add BPF_KPROBE_SYSCALL macro
  selftests/bpf: Test BPF_KPROBE_SYSCALL macro

 tools/lib/bpf/bpf_tracing.h                   | 39 +++++++++++++++++++
 .../selftests/bpf/prog_tests/kprobe_syscall.c | 37 ++++++++++++++++++
 .../selftests/bpf/progs/test_kprobe_syscall.c | 34 ++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_kprobe_syscall.c

--
2.30.2

[PATCH bpf-next v2 1/2] libbpf: Add BPF_KPROBE_SYSCALL macro

[Hengqi Chen]

Add syscall-specific variant of BPF_KPROBE named BPF_KPROBE_SYSCALL ([0]).
The new macro hides the underlying way of getting syscall input arguments.
With these new macros, the following code:

    SEC("kprobe/__x64_sys_close")
    int BPF_KPROBE(do_sys_close, struct pt_regs *regs)
    {
        int fd;

        fd = PT_REGS_PARM1_CORE(regs);
        /* do something with fd */
    }

can be written as:

    SEC("kprobe/__x64_sys_close")
    int BPF_KPROBE_SYSCALL(do_sys_close, int fd)
    {
        /* do something with fd */
    }

  [0] Closes: https://github.com/libbpf/libbpf/issues/425

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 tools/lib/bpf/bpf_tracing.h | 39 +++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
index cf980e54d331..a0b230320335 100644
--- a/tools/lib/bpf/bpf_tracing.h
+++ b/tools/lib/bpf/bpf_tracing.h
@@ -461,4 +461,43 @@ typeof(name(0)) name(struct pt_regs *ctx)				    \
 }									    \
 static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)

+#define ___bpf_syscall_args0() ctx
+#define ___bpf_syscall_args1(x) \
+	___bpf_syscall_args0(), (void *)PT_REGS_PARM1_CORE_SYSCALL(regs)
+#define ___bpf_syscall_args2(x, args...) \
+	___bpf_syscall_args1(args), (void *)PT_REGS_PARM2_CORE_SYSCALL(regs)
+#define ___bpf_syscall_args3(x, args...) \
+	___bpf_syscall_args2(args), (void *)PT_REGS_PARM3_CORE_SYSCALL(regs)
+#define ___bpf_syscall_args4(x, args...) \
+	___bpf_syscall_args3(args), (void *)PT_REGS_PARM4_CORE_SYSCALL(regs)
+#define ___bpf_syscall_args5(x, args...) \
+	___bpf_syscall_args4(args), (void *)PT_REGS_PARM5_CORE_SYSCALL(regs)
+#define ___bpf_syscall_args(args...) \
+	___bpf_apply(___bpf_syscall_args, ___bpf_narg(args))(args)
+
+/*
+ * BPF_KPROBE_SYSCALL is a variant of BPF_KPROBE, which is intended for
+ * tracing syscall functions, like __x64_sys_close. It hides the underlying
+ * platform-specific low-level way of getting syscall input arguments from
+ * struct pt_regs, and provides a familiar typed and named function arguments
+ * syntax and semantics of accessing syscall input parameters.
+ *
+ * Original struct pt_regs* context is preserved as 'ctx' argument. This might
+ * be necessary when using BPF helpers like bpf_perf_event_output().
+ */
+#define BPF_KPROBE_SYSCALL(name, args...)				    \
+name(struct pt_regs *ctx);						    \
+static __attribute__((always_inline)) typeof(name(0))			    \
+____##name(struct pt_regs *ctx, ##args);				    \
+typeof(name(0)) name(struct pt_regs *ctx)				    \
+{									    \
+	struct pt_regs *regs = (void *)PT_REGS_PARM1(ctx);		    \
+	_Pragma("GCC diagnostic push")					    \
+	_Pragma("GCC diagnostic ignored \"-Wint-conversion\"")		    \
+	return ____##name(___bpf_syscall_args(args));			    \
+	_Pragma("GCC diagnostic pop")					    \
+}									    \
+static __attribute__((always_inline)) typeof(name(0))			    \
+____##name(struct pt_regs *ctx, ##args)
+
 #endif
--
2.30.2

[PATCH bpf-next v2 2/2] selftests/bpf: Test BPF_KPROBE_SYSCALL macro

[Hengqi Chen]

Add tests for the newly added BPF_KPROBE_SYSCALL macro.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 .../selftests/bpf/prog_tests/kprobe_syscall.c | 37 +++++++++++++++++++
 .../selftests/bpf/progs/test_kprobe_syscall.c | 34 +++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_kprobe_syscall.c

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
new file mode 100644
index 000000000000..0ac89c987024
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Hengqi Chen */
+
+#include <test_progs.h>
+#include <sys/prctl.h>
+#include "test_kprobe_syscall.skel.h"
+
+void test_kprobe_syscall(void)
+{
+	struct test_kprobe_syscall *skel;
+	int err;
+
+	skel = test_kprobe_syscall__open();
+	if (!ASSERT_OK_PTR(skel, "test_kprobe_syscall__open"))
+		return;
+
+	skel->rodata->my_pid = getpid();
+
+	err = test_kprobe_syscall__load(skel);
+	if (!ASSERT_OK(err, "test_kprobe_syscall__load"))
+		goto cleanup;
+
+	err = test_kprobe_syscall__attach(skel);
+	if (!ASSERT_OK(err, "test_kprobe_syscall__attach"))
+		goto cleanup;
+
+	prctl(1, 2, 3, 4, 5);
+
+	ASSERT_EQ(skel->bss->option, 1, "BPF_KPROBE_SYSCALL failed");
+	ASSERT_EQ(skel->bss->arg2, 2, "BPF_KPROBE_SYSCALL failed");
+	ASSERT_EQ(skel->bss->arg3, 3, "BPF_KPROBE_SYSCALL failed");
+	ASSERT_EQ(skel->bss->arg4, 4, "BPF_KPROBE_SYSCALL failed");
+	ASSERT_EQ(skel->bss->arg5, 5, "BPF_KPROBE_SYSCALL failed");
+
+cleanup:
+	test_kprobe_syscall__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
new file mode 100644
index 000000000000..abd59c3d5b59
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Hengqi Chen */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_misc.h"
+
+const volatile pid_t my_pid = 0;
+int option = 0;
+unsigned long arg2 = 0;
+unsigned long arg3 = 0;
+unsigned long arg4 = 0;
+unsigned long arg5 = 0;
+
+SEC("kprobe/" SYS_PREFIX "sys_prctl")
+int BPF_KPROBE_SYSCALL(prctl_enter, int opt, unsigned long a2,
+		       unsigned long a3, unsigned long a4, unsigned long a5)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != my_pid)
+		return 0;
+
+	option = opt;
+	arg2 = a2;
+	arg3 = a3;
+	arg4 = a4;
+	arg5 = a5;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.30.2

Re: [PATCH bpf-next v2 1/2] libbpf: Add BPF_KPROBE_SYSCALL macro

[Andrii Nakryiko]

On Sun, Feb 6, 2022 at 5:41 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Add syscall-specific variant of BPF_KPROBE named BPF_KPROBE_SYSCALL ([0]).
> The new macro hides the underlying way of getting syscall input arguments.
> With these new macros, the following code:
>
>     SEC("kprobe/__x64_sys_close")
>     int BPF_KPROBE(do_sys_close, struct pt_regs *regs)
>     {
>         int fd;
>
>         fd = PT_REGS_PARM1_CORE(regs);
>         /* do something with fd */
>     }
>
> can be written as:
>
>     SEC("kprobe/__x64_sys_close")
>     int BPF_KPROBE_SYSCALL(do_sys_close, int fd)
>     {
>         /* do something with fd */
>     }
>
>   [0] Closes: https://github.com/libbpf/libbpf/issues/425
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  tools/lib/bpf/bpf_tracing.h | 39 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_tracing.h b/tools/lib/bpf/bpf_tracing.h
> index cf980e54d331..a0b230320335 100644
> --- a/tools/lib/bpf/bpf_tracing.h
> +++ b/tools/lib/bpf/bpf_tracing.h
> @@ -461,4 +461,43 @@ typeof(name(0)) name(struct pt_regs *ctx)                              \
>  }                                                                          \
>  static __always_inline typeof(name(0)) ____##name(struct pt_regs *ctx, ##args)
>
> +#define ___bpf_syscall_args0() ctx
> +#define ___bpf_syscall_args1(x) \
> +       ___bpf_syscall_args0(), (void *)PT_REGS_PARM1_CORE_SYSCALL(regs)
> +#define ___bpf_syscall_args2(x, args...) \
> +       ___bpf_syscall_args1(args), (void *)PT_REGS_PARM2_CORE_SYSCALL(regs)
> +#define ___bpf_syscall_args3(x, args...) \
> +       ___bpf_syscall_args2(args), (void *)PT_REGS_PARM3_CORE_SYSCALL(regs)
> +#define ___bpf_syscall_args4(x, args...) \
> +       ___bpf_syscall_args3(args), (void *)PT_REGS_PARM4_CORE_SYSCALL(regs)
> +#define ___bpf_syscall_args5(x, args...) \
> +       ___bpf_syscall_args4(args), (void *)PT_REGS_PARM5_CORE_SYSCALL(regs)
> +#define ___bpf_syscall_args(args...) \
> +       ___bpf_apply(___bpf_syscall_args, ___bpf_narg(args))(args)
> +

please keep each #define on a single line, it's much easier to follow
(and validate) the pattern that way

> +/*
> + * BPF_KPROBE_SYSCALL is a variant of BPF_KPROBE, which is intended for
> + * tracing syscall functions, like __x64_sys_close. It hides the underlying
> + * platform-specific low-level way of getting syscall input arguments from
> + * struct pt_regs, and provides a familiar typed and named function arguments
> + * syntax and semantics of accessing syscall input parameters.
> + *
> + * Original struct pt_regs* context is preserved as 'ctx' argument. This might
> + * be necessary when using BPF helpers like bpf_perf_event_output().
> + */
> +#define BPF_KPROBE_SYSCALL(name, args...)                                  \
> +name(struct pt_regs *ctx);                                                 \
> +static __attribute__((always_inline)) typeof(name(0))                      \
> +____##name(struct pt_regs *ctx, ##args);                                   \
> +typeof(name(0)) name(struct pt_regs *ctx)                                  \
> +{                                                                          \
> +       struct pt_regs *regs = (void *)PT_REGS_PARM1(ctx);                  \

See recent Ilya's patch set ([0]), not all architectures need this
unwrapping. We've abstracted this into PT_REGS_SYSCALL() macro, please
use it here.

  [0] https://patchwork.kernel.org/project/netdevbpf/list/?series=611184&state=*

> +       _Pragma("GCC diagnostic push")                                      \
> +       _Pragma("GCC diagnostic ignored \"-Wint-conversion\"")              \
> +       return ____##name(___bpf_syscall_args(args));                       \
> +       _Pragma("GCC diagnostic pop")                                       \
> +}                                                                          \
> +static __attribute__((always_inline)) typeof(name(0))                      \
> +____##name(struct pt_regs *ctx, ##args)
> +
>  #endif
> --
> 2.30.2

Re: [PATCH bpf-next v2 2/2] selftests/bpf: Test BPF_KPROBE_SYSCALL macro

[Andrii Nakryiko]

On Sun, Feb 6, 2022 at 5:41 AM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Add tests for the newly added BPF_KPROBE_SYSCALL macro.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/kprobe_syscall.c | 37 +++++++++++++++++++
>  .../selftests/bpf/progs/test_kprobe_syscall.c | 34 +++++++++++++++++
>  2 files changed, 71 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
> new file mode 100644
> index 000000000000..0ac89c987024
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022 Hengqi Chen */
> +
> +#include <test_progs.h>
> +#include <sys/prctl.h>
> +#include "test_kprobe_syscall.skel.h"
> +
> +void test_kprobe_syscall(void)
> +{
> +       struct test_kprobe_syscall *skel;
> +       int err;
> +
> +       skel = test_kprobe_syscall__open();
> +       if (!ASSERT_OK_PTR(skel, "test_kprobe_syscall__open"))
> +               return;
> +
> +       skel->rodata->my_pid = getpid();
> +
> +       err = test_kprobe_syscall__load(skel);
> +       if (!ASSERT_OK(err, "test_kprobe_syscall__load"))
> +               goto cleanup;
> +
> +       err = test_kprobe_syscall__attach(skel);
> +       if (!ASSERT_OK(err, "test_kprobe_syscall__attach"))
> +               goto cleanup;
> +
> +       prctl(1, 2, 3, 4, 5);
> +
> +       ASSERT_EQ(skel->bss->option, 1, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->arg2, 2, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->arg3, 3, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->arg4, 4, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->arg5, 5, "BPF_KPROBE_SYSCALL failed");
> +
> +cleanup:
> +       test_kprobe_syscall__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
> new file mode 100644
> index 000000000000..abd59c3d5b59
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2022 Hengqi Chen */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_core_read.h>
> +#include "bpf_misc.h"
> +
> +const volatile pid_t my_pid = 0;
> +int option = 0;
> +unsigned long arg2 = 0;
> +unsigned long arg3 = 0;
> +unsigned long arg4 = 0;
> +unsigned long arg5 = 0;
> +
> +SEC("kprobe/" SYS_PREFIX "sys_prctl")
> +int BPF_KPROBE_SYSCALL(prctl_enter, int opt, unsigned long a2,
> +                      unsigned long a3, unsigned long a4, unsigned long a5)
> +{
> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
> +
> +       if (pid != my_pid)
> +               return 0;
> +
> +       option = opt;
> +       arg2 = a2;
> +       arg3 = a3;
> +       arg4 = a4;
> +       arg5 = a5;
> +       return 0;
> +}
> +

Let's add all this into progs/bpf_syscall_macro.c and
prog_tests/test_bpf_syscall_macro.c, instead of adding a new selftest.
They are closely related anyways.

> +char _license[] SEC("license") = "GPL";
> --
> 2.30.2