diff mbox series

[bpf-next,5/5] selftests/bpf: add tests for sleepable kprobes and uprobes

Message ID d460d5b8a184a9d431efa3a016f56389415a1fc6.1651103126.git.delyank@fb.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series sleepable uprobe support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next
bpf/vmtest-bpf-next-PR fail merge-conflict

Commit Message

Delyan Kratunov April 28, 2022, 4:53 p.m. UTC 
Add tests that ensure sleepable kprobe programs cannot attach.

Also attach both sleepable and non-sleepable uprobe programs to the same
location (i.e. same bpf_prog_array).

Signed-off-by: Delyan Kratunov <delyank@fb.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 35 +++++++++++++++
 .../selftests/bpf/progs/test_attach_probe.c   | 44 +++++++++++++++++++
 2 files changed, 79 insertions(+)

Comments

Andrii Nakryiko April 28, 2022, 6:41 p.m. UTC | #1 
On Thu, Apr 28, 2022 at 9:54 AM Delyan Kratunov <delyank@fb.com> wrote:
>
> Add tests that ensure sleepable kprobe programs cannot attach.
>
> Also attach both sleepable and non-sleepable uprobe programs to the same
> location (i.e. same bpf_prog_array).
>

Yep, great thinking! I left a few comments below, otherwise looks good.

> Signed-off-by: Delyan Kratunov <delyank@fb.com>
> ---
>  .../selftests/bpf/prog_tests/attach_probe.c   | 35 +++++++++++++++
>  .../selftests/bpf/progs/test_attach_probe.c   | 44 +++++++++++++++++++
>  2 files changed, 79 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> index c0c6d410751d..c5c601537eea 100644
> --- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -17,6 +17,12 @@ static void trigger_func2(void)
>         asm volatile ("");
>  }
>
> +/* attach point for byname sleepable uprobe */
> +static void trigger_func3(void)
> +{
> +       asm volatile ("");
> +}
> +
>  void test_attach_probe(void)
>  {
>         DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
> @@ -143,6 +149,28 @@ void test_attach_probe(void)
>         if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname2, "attach_uretprobe_byname2"))
>                 goto cleanup;
>
> +       /* sleepable kprobes should not attach successfully */
> +       skel->links.handle_kprobe_sleepable = bpf_program__attach(skel->progs.handle_kprobe_sleepable);
> +       if (!ASSERT_NULL(skel->links.handle_kprobe_sleepable, "attach_kprobe_sleepable"))

we have ASSERT_ERR_PTR() which is more in line with ASSERT_OK_PTR(),
let's use that one.

With dropping SEC("kprobe.s") you'll have to do one extra step to make
sure that handle_kprobe_sleepable is actually sleepable program during
BPF verification. Please use bpf_program__set_flags() before load step
for that.

> +               goto cleanup;
> +
> +       /* test sleepable uprobe and uretprobe variants */
> +       skel->links.handle_uprobe_byname3_sleepable = bpf_program__attach(skel->progs.handle_uprobe_byname3_sleepable);
> +       if (!ASSERT_OK_PTR(skel->links.handle_uprobe_byname3_sleepable, "attach_uprobe_byname3_sleepable"))
> +               goto cleanup;
> +

[...]

> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> index af994d16bb10..265a23af74d4 100644
> --- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -14,6 +14,10 @@ int uprobe_byname_res = 0;
>  int uretprobe_byname_res = 0;
>  int uprobe_byname2_res = 0;
>  int uretprobe_byname2_res = 0;
> +int uprobe_byname3_sleepable_res = 0;
> +int uprobe_byname3_res = 0;
> +int uretprobe_byname3_sleepable_res = 0;
> +int uretprobe_byname3_res = 0;
>
>  SEC("kprobe/sys_nanosleep")
>  int handle_kprobe(struct pt_regs *ctx)
> @@ -22,6 +26,13 @@ int handle_kprobe(struct pt_regs *ctx)
>         return 0;
>  }
>
> +SEC("kprobe.s/sys_nanosleep")

can you please leave comment here that this is supposed to fail to be
attached? It took me a bit to notice that you do negative test with
this program

> +int handle_kprobe_sleepable(struct pt_regs *ctx)
> +{
> +       kprobe_res = 2;
> +       return 0;
> +}
> +

[...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
index c0c6d410751d..c5c601537eea 100644
--- a/tools/testing/selftests/bpf/prog_tests/attach_probe.c
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -17,6 +17,12 @@  static void trigger_func2(void)
 	asm volatile ("");
 }
 
+/* attach point for byname sleepable uprobe */
+static void trigger_func3(void)
+{
+	asm volatile ("");
+}
+
 void test_attach_probe(void)
 {
 	DECLARE_LIBBPF_OPTS(bpf_uprobe_opts, uprobe_opts);
@@ -143,6 +149,28 @@  void test_attach_probe(void)
 	if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname2, "attach_uretprobe_byname2"))
 		goto cleanup;
 
+	/* sleepable kprobes should not attach successfully */
+	skel->links.handle_kprobe_sleepable = bpf_program__attach(skel->progs.handle_kprobe_sleepable);
+	if (!ASSERT_NULL(skel->links.handle_kprobe_sleepable, "attach_kprobe_sleepable"))
+		goto cleanup;
+
+	/* test sleepable uprobe and uretprobe variants */
+	skel->links.handle_uprobe_byname3_sleepable = bpf_program__attach(skel->progs.handle_uprobe_byname3_sleepable);
+	if (!ASSERT_OK_PTR(skel->links.handle_uprobe_byname3_sleepable, "attach_uprobe_byname3_sleepable"))
+		goto cleanup;
+
+	skel->links.handle_uprobe_byname3 = bpf_program__attach(skel->progs.handle_uprobe_byname3);
+	if (!ASSERT_OK_PTR(skel->links.handle_uprobe_byname3, "attach_uprobe_byname3"))
+		goto cleanup;
+
+	skel->links.handle_uretprobe_byname3_sleepable = bpf_program__attach(skel->progs.handle_uretprobe_byname3_sleepable);
+	if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname3_sleepable, "attach_uretprobe_byname3_sleepable"))
+		goto cleanup;
+
+	skel->links.handle_uretprobe_byname3 = bpf_program__attach(skel->progs.handle_uretprobe_byname3);
+	if (!ASSERT_OK_PTR(skel->links.handle_uretprobe_byname3, "attach_uretprobe_byname3"))
+		goto cleanup;
+
 	/* trigger & validate kprobe && kretprobe */
 	usleep(1);
 
@@ -156,6 +184,9 @@  void test_attach_probe(void)
 	/* trigger & validate uprobe attached by name */
 	trigger_func2();
 
+	/* trigger & validate sleepable uprobe attached by name */
+	trigger_func3();
+
 	ASSERT_EQ(skel->bss->kprobe_res, 1, "check_kprobe_res");
 	ASSERT_EQ(skel->bss->kretprobe_res, 2, "check_kretprobe_res");
 	ASSERT_EQ(skel->bss->uprobe_res, 3, "check_uprobe_res");
@@ -164,6 +195,10 @@  void test_attach_probe(void)
 	ASSERT_EQ(skel->bss->uretprobe_byname_res, 6, "check_uretprobe_byname_res");
 	ASSERT_EQ(skel->bss->uprobe_byname2_res, 7, "check_uprobe_byname2_res");
 	ASSERT_EQ(skel->bss->uretprobe_byname2_res, 8, "check_uretprobe_byname2_res");
+	ASSERT_EQ(skel->bss->uprobe_byname3_sleepable_res, 9, "check_uprobe_byname3_sleepable_res");
+	ASSERT_EQ(skel->bss->uprobe_byname3_res, 10, "check_uprobe_byname3_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname3_sleepable_res, 11, "check_uretprobe_byname3_sleepable_res");
+	ASSERT_EQ(skel->bss->uretprobe_byname3_res, 12, "check_uretprobe_byname3_res");
 
 cleanup:
 	test_attach_probe__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
index af994d16bb10..265a23af74d4 100644
--- a/tools/testing/selftests/bpf/progs/test_attach_probe.c
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -14,6 +14,10 @@  int uprobe_byname_res = 0;
 int uretprobe_byname_res = 0;
 int uprobe_byname2_res = 0;
 int uretprobe_byname2_res = 0;
+int uprobe_byname3_sleepable_res = 0;
+int uprobe_byname3_res = 0;
+int uretprobe_byname3_sleepable_res = 0;
+int uretprobe_byname3_res = 0;
 
 SEC("kprobe/sys_nanosleep")
 int handle_kprobe(struct pt_regs *ctx)
@@ -22,6 +26,13 @@  int handle_kprobe(struct pt_regs *ctx)
 	return 0;
 }
 
+SEC("kprobe.s/sys_nanosleep")
+int handle_kprobe_sleepable(struct pt_regs *ctx)
+{
+	kprobe_res = 2;
+	return 0;
+}
+
 SEC("kretprobe/sys_nanosleep")
 int BPF_KRETPROBE(handle_kretprobe)
 {
@@ -76,4 +87,37 @@  int handle_uretprobe_byname2(struct pt_regs *ctx)
 	return 0;
 }
 
+SEC("uprobe.s//proc/self/exe:trigger_func3")
+int handle_uprobe_byname3_sleepable(struct pt_regs *ctx)
+{
+	uprobe_byname3_sleepable_res = 9;
+	return 0;
+}
+
+/**
+ * same target as the uprobe.s above to force sleepable and non-sleepable
+ * programs in the same bpf_prog_array
+ */
+SEC("uprobe//proc/self/exe:trigger_func3")
+int handle_uprobe_byname3(struct pt_regs *ctx)
+{
+	uprobe_byname3_res = 10;
+	return 0;
+}
+
+SEC("uretprobe.s//proc/self/exe:trigger_func3")
+int handle_uretprobe_byname3_sleepable(struct pt_regs *ctx)
+{
+	uretprobe_byname3_sleepable_res = 11;
+	return 0;
+}
+
+SEC("uretprobe//proc/self/exe:trigger_func3")
+int handle_uretprobe_byname3(struct pt_regs *ctx)
+{
+	uretprobe_byname3_res = 12;
+	return 0;
+}
+
+
 char _license[] SEC("license") = "GPL";