[bpf-next,v2] selftests/bpf: Make sure libbpf doesn't enforce the signature of a func pointer.

Commit Message

Kui-Feng Lee April 4, 2024, 11:23 p.m. UTC

The verifier in the kernel ensures that the struct_ops operators behave
correctly by checking that they access parameters and context
appropriately. The verifier will approve a program as long as it correctly
accesses the context/parameters, regardless of its function signature. In
contrast, libbpf should not verify the signature of function pointers and
functions to enable flexibility in loading various implementations of an
operator even if the signature of the function pointer does not match those
in the implementations or the kernel.

With this flexibility, user space applications can adapt to different
kernel versions by loading a specific implementation of an operator based
on feature detection.

This is a follow-up of the commit c911fc61a7ce ("libbpf: Skip zeroed or
null fields if not found in the kernel type.")

Signed-off-by: Kui-Feng Lee <[e-mail]>

---
Major changes from v1:

  - Rephrase commit logs to remove the description of trial-and-error
    approach.

  - Fix imprecise description of the verifier. It checks the behavior of
    the bpf programs in accessing the context/parameters, not signatures.

v1: https://lore.kernel.org/all/[e-mail]/
---
 .../bpf/prog_tests/test_struct_ops_module.c   | 24 +++++++++++++++++++
 .../selftests/bpf/progs/struct_ops_module.c   | 13 ++++++++++
 2 files changed, 37 insertions(+)

Comments

[e-mail] April 5, 2024, 5:40 p.m. UTC | #1

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[e-mail]>:

On Thu,  4 Apr 2024 16:23:42 -0700 you wrote:
> The verifier in the kernel ensures that the struct_ops operators behave
> correctly by checking that they access parameters and context
> appropriately. The verifier will approve a program as long as it correctly
> accesses the context/parameters, regardless of its function signature. In
> contrast, libbpf should not verify the signature of function pointers and
> functions to enable flexibility in loading various implementations of an
> operator even if the signature of the function pointer does not match those
> in the implementations or the kernel.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests/bpf: Make sure libbpf doesn't enforce the signature of a func pointer.
    https://git.kernel.org/bpf/bpf-next/c/ba0cbe2bb4ab

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
index 098776d00ab4..7cf2b9ddd3e1 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
@@ -138,11 +138,35 @@  static void test_struct_ops_not_zeroed(void)
 	struct_ops_module__destroy(skel);
 }
 
+/* The signature of an implementation might not match the signature of the
+ * function pointer prototype defined in the BPF program. This mismatch
+ * should be allowed as long as the behavior of the operator program
+ * adheres to the signature in the kernel. Libbpf should not enforce the
+ * signature; rather, let the kernel verifier handle the enforcement.
+ */
+static void test_struct_ops_incompatible(void)
+{
+	struct struct_ops_module *skel;
+	struct bpf_link *link;
+
+	skel = struct_ops_module__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(skel->maps.testmod_incompatible);
+	if (ASSERT_OK_PTR(link, "attach_struct_ops"))
+		bpf_link__destroy(link);
+
+	struct_ops_module__destroy(skel);
+}
+
 void serial_test_struct_ops_module(void)
 {
 	if (test__start_subtest("test_struct_ops_load"))
 		test_struct_ops_load();
 	if (test__start_subtest("test_struct_ops_not_zeroed"))
 		test_struct_ops_not_zeroed();
+	if (test__start_subtest("test_struct_ops_incompatible"))
+		test_struct_ops_incompatible();
 }
 
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
index 86e1e50c5531..63b065dae002 100644
--- a/tools/testing/selftests/bpf/progs/struct_ops_module.c
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -68,3 +68,16 @@  struct bpf_testmod_ops___zeroed testmod_zeroed = {
 	.test_1 = (void *)test_1,
 	.test_2 = (void *)test_2_v2,
 };
+
+struct bpf_testmod_ops___incompatible {
+	int (*test_1)(void);
+	void (*test_2)(int *a);
+	int data;
+};
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops___incompatible testmod_incompatible = {
+	.test_1 = (void *)test_1,
+	.test_2 = (void *)test_2,
+	.data = 3,
+};