From: Kui-Feng Lee <thinker.li@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org, martin.lau@linux.dev,
song@kernel.org, kernel-team@meta.com, andrii@kernel.org
Cc: sinquersw@gmail.com, kuifeng@meta.com,
Kui-Feng Lee <thinker.li@gmail.com>
Subject: [PATCH bpf-next v2 3/3] selftests/bpf: Accept extra arguments if they are not used.
Date: Wed, 13 Mar 2024 14:41:39 -0700 [thread overview]
Message-ID: <20240313214139.685112-4-thinker.li@gmail.com> (raw)
In-Reply-To: <20240313214139.685112-1-thinker.li@gmail.com>
A struct_ops program can have more arguments than what the corresponding
function pointer in the kernel has if the program doesn't access the
arguments. For example, a struct_ops operator may have 2 arguments, but
your program may defined with 3 or more arguments. It is acceptable as long
as the program doesn't use these arguments.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
.../bpf/prog_tests/test_struct_ops_module.c | 56 +++++++++++++++++++
| 49 ++++++++++++++++
2 files changed, 105 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_extra_arg.c
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..a146bf079a60 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
@@ -4,6 +4,7 @@
#include <time.h>
#include "struct_ops_module.skel.h"
+#include "struct_ops_extra_arg.skel.h"
static void check_map_info(struct bpf_map_info *info)
{
@@ -138,11 +139,66 @@ static void test_struct_ops_not_zeroed(void)
struct_ops_module__destroy(skel);
}
+/* Handle BPF programs with additional arguments that don't appear in the
+ * function pointer prototype in the kernel.
+ */
+static void test_struct_ops_extra_arg(void)
+{
+ struct struct_ops_extra_arg *skel;
+ int err;
+
+ /* test_extra_arg() has an extra argument, so testmod_1 should fail
+ * to load.
+ *
+ * Since extra_arg is used in test_extra_arg(), it should be
+ * rejected by the verifier.
+ */
+ skel = struct_ops_extra_arg__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_extra_arg_open_extra_arg"))
+ return;
+
+ err = struct_ops_extra_arg__load(skel);
+ ASSERT_ERR(err, "struct_ops_extra_arg_load_extra_arg");
+
+ struct_ops_extra_arg__destroy(skel);
+
+ /* Switch to test_2() */
+ skel = struct_ops_extra_arg__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_extra_arg_open"))
+ return;
+
+ skel->struct_ops.testmod_1->test_2 = skel->progs.test_2;
+
+ err = struct_ops_extra_arg__load(skel);
+ ASSERT_OK(err, "struct_ops_extra_arg_load");
+
+ struct_ops_extra_arg__destroy(skel);
+
+ /* Switch to test_extra_arg_unused()
+ *
+ * Since extra_arg is never used, it should be accepted by the
+ * verifier. The verifier would accept a program with extra
+ * arguments as long as they are not used.
+ */
+ skel = struct_ops_extra_arg__open();
+ if (!ASSERT_OK_PTR(skel, "struct_ops_extra_arg_open_unused"))
+ return;
+
+ skel->struct_ops.testmod_1->test_2 = skel->progs.test_extra_arg_unused;
+
+ err = struct_ops_extra_arg__load(skel);
+ ASSERT_OK(err, "struct_ops_extra_arg_load_unused");
+
+ struct_ops_extra_arg__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_extra_arg"))
+ test_struct_ops_extra_arg();
}
--git a/tools/testing/selftests/bpf/progs/struct_ops_extra_arg.c b/tools/testing/selftests/bpf/progs/struct_ops_extra_arg.c
new file mode 100644
index 000000000000..4b73b279ad22
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_extra_arg.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+int test_1_result = 0;
+int test_2_result = 0;
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1)
+{
+ test_1_result = 0xdeadbeef;
+ return 0;
+}
+
+SEC("?struct_ops/test_2")
+void BPF_PROG(test_2, int a, int b)
+{
+ test_2_result = a + b;
+}
+
+SEC("?struct_ops/test_extra_arg")
+void BPF_PROG(test_extra_arg, int a, int b, int extra_arg)
+{
+ /* Accessing extra_arg will cause a verifier error since it
+ * accesses the context data beyond the size of the context.
+ */
+ test_2_result = a + b + extra_arg + 3;
+}
+
+SEC("?struct_ops/test_extra_arg_unused")
+void BPF_PROG(test_extra_arg_unused, int a, int b, int extra_arg)
+{
+ /* The extra_arg is not used, so it should not cause a verifier
+ * error.
+ */
+ test_2_result = a + b + 3;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_1 = {
+ .test_1 = (void *)test_1,
+ .test_2 = (void *)test_extra_arg,
+ .data = 0x1,
+};
--
2.34.1
next prev parent reply other threads:[~2024-03-13 21:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-13 21:41 [PATCH bpf-next v2 0/3] Ignore additional fields in the struct_ops maps in an updated version Kui-Feng Lee
2024-03-13 21:41 ` [PATCH bpf-next v2 1/3] libbpf: Skip zeroed or null fields if not found in the kernel type Kui-Feng Lee
2024-03-13 21:41 ` [PATCH bpf-next v2 2/3] selftests/bpf: Ensure libbpf skip all-zeros fields of struct_ops maps Kui-Feng Lee
2024-03-13 21:41 ` Kui-Feng Lee [this message]
2024-03-14 20:59 ` [PATCH bpf-next v2 0/3] Ignore additional fields in the struct_ops maps in an updated version Andrii Nakryiko
2024-03-15 23:44 ` Kui-Feng Lee
2024-03-18 18:34 ` Andrii Nakryiko
2024-03-18 21:08 ` Kui-Feng Lee
2024-03-14 21:00 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240313214139.685112-4-thinker.li@gmail.com \
--to=thinker.li@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=kernel-team@meta.com \
--cc=kuifeng@meta.com \
--cc=martin.lau@linux.dev \
--cc=sinquersw@gmail.com \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.