[RFC,bpf-next,2/3] selftests/bpf: Add uretprobe syscall test

Commit Message

Jiri Olsa March 18, 2024, 9:31 a.m. UTC

Add uretprobe syscall test and compare register values before
and after the uretprobe is hit. Also compare the register values
seen from attached bpf program.

Signed-off-by: Jiri Olsa <[e-mail]>
---
 tools/testing/selftests/bpf/Makefile          | 13 ++-
 .../bpf/prog_tests/arch/x86/uprobe_syscall.S  | 89 +++++++++++++++++++
 .../selftests/bpf/prog_tests/uprobe_syscall.c | 84 +++++++++++++++++
 .../selftests/bpf/progs/uprobe_syscall.c      | 15 ++++
 4 files changed, 200 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S
 create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
 create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c

Comments

Andrii Nakryiko March 19, 2024, 1:16 a.m. UTC | #1

On Mon, Mar 18, 2024 at 2:32 AM Jiri Olsa <[e-mail]> wrote:
>
> Add uretprobe syscall test and compare register values before
> and after the uretprobe is hit. Also compare the register values
> seen from attached bpf program.
>
> Signed-off-by: Jiri Olsa <[e-mail]>
> ---
>  tools/testing/selftests/bpf/Makefile          | 13 ++-
>  .../bpf/prog_tests/arch/x86/uprobe_syscall.S  | 89 +++++++++++++++++++
>  .../selftests/bpf/prog_tests/uprobe_syscall.c | 84 +++++++++++++++++
>  .../selftests/bpf/progs/uprobe_syscall.c      | 15 ++++
>  4 files changed, 200 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
>  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
>

Can all the above be achieved with inline assembly inside .c files? It
would probably simplify logistics overall. We can guard with
arch-specific #ifdefs, of course.

[...]

> diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall.c b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
> new file mode 100644
> index 000000000000..0cc7e8761410
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <string.h>
> +
> +struct pt_regs regs;
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("uretprobe//proc/self/exe:uprobe_syscall_arch_test")
> +int uretprobe(struct pt_regs *ctx)
> +{
> +       memcpy(&regs, ctx, sizeof(regs));

nit: please use __builtin_memcpy(), given this is BPF code. And we
don't need string.h include.

> +       return 0;
> +}
> --
> 2.44.0
>

Jiri Olsa March 19, 2024, 11:09 a.m. UTC | #2

On Mon, Mar 18, 2024 at 06:16:09PM -0700, Andrii Nakryiko wrote:
> On Mon, Mar 18, 2024 at 2:32 AM Jiri Olsa <[e-mail]> wrote:
> >
> > Add uretprobe syscall test and compare register values before
> > and after the uretprobe is hit. Also compare the register values
> > seen from attached bpf program.
> >
> > Signed-off-by: Jiri Olsa <[e-mail]>
> > ---
> >  tools/testing/selftests/bpf/Makefile          | 13 ++-
> >  .../bpf/prog_tests/arch/x86/uprobe_syscall.S  | 89 +++++++++++++++++++
> >  .../selftests/bpf/prog_tests/uprobe_syscall.c | 84 +++++++++++++++++
> >  .../selftests/bpf/progs/uprobe_syscall.c      | 15 ++++
> >  4 files changed, 200 insertions(+), 1 deletion(-)
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S
> >  create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> >  create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> >
> 
> Can all the above be achieved with inline assembly inside .c files? It
> would probably simplify logistics overall. We can guard with
> arch-specific #ifdefs, of course.

ok, probably yes.. I'll check

> 
> [...]
> 
> > diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall.c b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
> > new file mode 100644
> > index 000000000000..0cc7e8761410
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
> > @@ -0,0 +1,15 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "vmlinux.h"
> > +#include <bpf/bpf_helpers.h>
> > +#include <string.h>
> > +
> > +struct pt_regs regs;
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("uretprobe//proc/self/exe:uprobe_syscall_arch_test")
> > +int uretprobe(struct pt_regs *ctx)
> > +{
> > +       memcpy(&regs, ctx, sizeof(regs));
> 
> nit: please use __builtin_memcpy(), given this is BPF code. And we
> don't need string.h include.

right, thanks

jirka

> 
> > +       return 0;
> > +}
> > --
> > 2.44.0
> >

Patch

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 3b9eb40d6343..e425a946276b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -490,6 +490,9 @@  TRUNNER_TEST_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.test.o,	\
 				 $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/*.c)))
 TRUNNER_EXTRA_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o,		\
 				 $$(filter %.c,$(TRUNNER_EXTRA_SOURCES)))
+TRUNNER_ASM_OBJS := $$(patsubst %.S,$$(TRUNNER_OUTPUT)/%.arch.o,	\
+				 $$(notdir $$(wildcard $(TRUNNER_TESTS_DIR)/arch/$(SRCARCH)/*.S)))
+
 TRUNNER_EXTRA_HDRS := $$(filter %.h,$(TRUNNER_EXTRA_SOURCES))
 TRUNNER_TESTS_HDR := $(TRUNNER_TESTS_DIR)/tests.h
 TRUNNER_BPF_SRCS := $$(notdir $$(wildcard $(TRUNNER_BPF_PROGS_DIR)/*.c))
@@ -597,6 +600,13 @@  $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
 	$$(call msg,EXT-OBJ,$(TRUNNER_BINARY),$$@)
 	$(Q)$$(CC) $$(CFLAGS) -c $$< $$(LDLIBS) -o $$@
 
+$(TRUNNER_ASM_OBJS): $(TRUNNER_OUTPUT)/%.arch.o:			\
+		       $(TRUNNER_TESTS_DIR)/arch/$(SRCARCH)/%.S		\
+		       $(TRUNNER_TESTS_HDR)				\
+		       $$(BPFOBJ) | $(TRUNNER_OUTPUT)
+	$$(call msg,ASM-OBJ,$(TRUNNER_BINARY),$$@)
+	$(Q)$$(CC) $$(CFLAGS) -c $$< $$(LDLIBS) -o $$@
+
 # non-flavored in-srctree builds receive special treatment, in particular, we
 # do not need to copy extra resources (see e.g. test_btf_dump_case())
 $(TRUNNER_BINARY)-extras: $(TRUNNER_EXTRA_FILES) | $(TRUNNER_OUTPUT)
@@ -606,7 +616,8 @@  ifneq ($2:$(OUTPUT),:$(shell pwd))
 endif
 
 $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)			\
-			     $(TRUNNER_EXTRA_OBJS) $$(BPFOBJ)		\
+			     $(TRUNNER_EXTRA_OBJS) $(TRUNNER_ASM_OBJS)	\
+			     $$(BPFOBJ)					\
 			     $(RESOLVE_BTFIDS)				\
 			     $(TRUNNER_BPFTOOL)				\
 			     | $(TRUNNER_BINARY)-extras
diff --git a/tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S b/tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S
new file mode 100644
index 000000000000..bcbad218c4d6
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/arch/x86/uprobe_syscall.S
@@ -0,0 +1,89 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef ASM_NL
+#define ASM_NL	 ;
+#endif
+
+#define SYM_ENTRY(name)			\
+	.globl name ASM_NL		\
+	name:
+
+#define SYM_END(name)			\
+	.type name STT_FUNC ASM_NL	\
+	.size name, .-name ASM_NL
+
+.code64
+.section .text, "ax"
+
+SYM_ENTRY(uprobe_syscall_arch_test)
+	movq $0xdeadbeef, %rax
+	ret
+SYM_END(uprobe_syscall_arch_test)
+
+.globl uprobe_syscall_arch
+uprobe_syscall_arch:
+	movq %r15,   0(%rdi)
+	movq %r14,   8(%rdi)
+	movq %r13,  16(%rdi)
+	movq %r12,  24(%rdi)
+	movq %rbp,  32(%rdi)
+	movq %rbx,  40(%rdi)
+	movq %r11,  48(%rdi)
+	movq %r10,  56(%rdi)
+	movq  %r9,  64(%rdi)
+	movq  %r8,  72(%rdi)
+	movq %rax,  80(%rdi)
+	movq %rcx,  88(%rdi)
+	movq %rdx,  96(%rdi)
+	movq %rsi, 104(%rdi)
+	movq %rdi, 112(%rdi)
+	movq   $0, 120(%rdi) /* orig_rax */
+	movq   $0, 128(%rdi) /* rip      */
+	movq   $0, 136(%rdi) /* cs       */
+
+	pushf
+	pop %rax
+
+	movq %rax, 144(%rdi) /* eflags   */
+	movq %rsp, 152(%rdi) /* rsp      */
+	movq   $0, 160(%rdi) /* ss       */
+
+	pushq %rsi
+	call uprobe_syscall_arch_test
+
+	/* store return value and get second argument pointer  to rax */
+	pushq %rax
+	movq 8(%rsp), %rax
+
+	movq %r15,   0(%rax)
+	movq %r14,   8(%rax)
+	movq %r13,  16(%rax)
+	movq %r12,  24(%rax)
+	movq %rbp,  32(%rax)
+	movq %rbx,  40(%rax)
+	movq %r11,  48(%rax)
+	movq %r10,  56(%rax)
+	movq  %r9,  64(%rax)
+	movq  %r8,  72(%rax)
+	movq %rcx,  88(%rax)
+	movq %rdx,  96(%rax)
+	movq %rsi, 104(%rax)
+	movq %rdi, 112(%rax)
+	movq   $0, 120(%rax) /* orig_rax */
+	movq   $0, 128(%rax) /* rip      */
+	movq   $0, 136(%rax) /* cs       */
+
+	pop %rax
+	pop %rsi
+	movq %rax,  80(%rsi)
+
+	pushf
+	pop %rax
+
+	movq %rax, 144(%rsi) /* eflags   */
+	movq %rsp, 152(%rsi) /* rsp      */
+	movq   $0, 160(%rsi) /* ss       */
+
+	ret
+
+.section .note.GNU-stack,"",@progbits
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
new file mode 100644
index 000000000000..0df205fea957
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
@@ -0,0 +1,84 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+
+#ifdef __x86_64__
+
+#include <unistd.h>
+#include <asm/ptrace.h>
+#include "uprobe_syscall.skel.h"
+
+extern int uprobe_syscall_arch(struct pt_regs *before, struct pt_regs *after);
+
+static void test_uretprobe(void)
+{
+	struct pt_regs before = {}, after = {};
+	unsigned long *pb = (unsigned long *) &before;
+	unsigned long *pa = (unsigned long *) &after;
+	unsigned long *prog_regs;
+	struct uprobe_syscall *skel = NULL;
+	unsigned int i, cnt;
+	int err;
+
+	skel = uprobe_syscall__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "uprobe_syscall__open_and_load"))
+		goto cleanup;
+
+	err = uprobe_syscall__attach(skel);
+	if (!ASSERT_OK(err, "uprobe_syscall__attach"))
+		goto cleanup;
+
+	uprobe_syscall_arch(&before, &after);
+
+	prog_regs = (unsigned long *) &skel->bss->regs;
+	cnt = sizeof(before)/sizeof(*pb);
+
+	for (i = 0; i < cnt; i++) {
+		unsigned int offset = i * sizeof(unsigned long);
+
+		/*
+		 * Check register before and after uprobe_syscall_arch_test call
+		 * that triggers the uretprobe.
+		 */
+		switch (offset) {
+		case offsetof(struct pt_regs, rax):
+			ASSERT_EQ(pa[i], 0xdeadbeef, "return value");
+			break;
+		default:
+			if (!ASSERT_EQ(pb[i], pa[i], "register before-after value check"))
+				fprintf(stdout, "failed register offset %u\n", offset);
+		}
+
+		/*
+		 * Check register seen from bpf program and register after
+		 * uprobe_syscall_arch_test call
+		 */
+		switch (offset) {
+		/*
+		 * These will be different (not set in uprobe_syscall_arch),
+		 * we don't care.
+		 */
+		case offsetof(struct pt_regs, orig_rax):
+		case offsetof(struct pt_regs, rip):
+		case offsetof(struct pt_regs, cs):
+		case offsetof(struct pt_regs, rsp):
+		case offsetof(struct pt_regs, ss):
+			break;
+		default:
+			if (!ASSERT_EQ(prog_regs[i], pa[i], "register prog-after value check"))
+				fprintf(stdout, "failed register offset %u\n", offset);
+		}
+	}
+
+cleanup:
+	uprobe_syscall__destroy(skel);
+}
+#else
+static void test_uretprobe(void) { }
+#endif
+
+void test_uprobe_syscall(void)
+{
+	if (test__start_subtest("uretprobe"))
+		test_uretprobe();
+}
diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall.c b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
new file mode 100644
index 000000000000..0cc7e8761410
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/uprobe_syscall.c
@@ -0,0 +1,15 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <string.h>
+
+struct pt_regs regs;
+
+char _license[] SEC("license") = "GPL";
+
+SEC("uretprobe//proc/self/exe:uprobe_syscall_arch_test")
+int uretprobe(struct pt_regs *ctx)
+{
+	memcpy(&regs, ctx, sizeof(regs));
+	return 0;
+}