* [PATCH bpf v2 1/3] bpf: fix kfunc btf caching for modules
2024-10-10 13:27 [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier Toke Høiland-Jørgensen
@ 2024-10-10 13:27 ` Toke Høiland-Jørgensen
2024-10-10 13:27 ` [PATCH bpf v2 2/3] selftests/bpf: Provide a generic [un]load_module helper Toke Høiland-Jørgensen
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2024-10-10 13:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Kumar Kartikeya Dwivedi
Cc: Simon Sundberg, bpf, netdev, Toke Høiland-Jørgensen
The verifier contains a cache for looking up module BTF objects when
calling kfuncs defined in modules. This cache uses a 'struct
bpf_kfunc_btf_tab', which contains a sorted list of BTF objects that
were already seen in the current verifier run, and the BTF objects are
looked up by the offset stored in the relocated call instruction using
bsearch().
The first time a given offset is seen, the module BTF is loaded from the
file descriptor passed in by libbpf, and stored into the cache. However,
there's a bug in the code storing the new entry: it stores a pointer to
the new cache entry, then calls sort() to keep the cache sorted for the
next lookup using bsearch(), and then returns the entry that was just
stored through the stored pointer. However, because sort() modifies the
list of entries in place *by value*, the stored pointer may no longer
point to the right entry, in which case the wrong BTF object will be
returned.
The end result of this is an intermittent bug where, if a BPF program
calls two functions with the same signature in two different modules,
the function from the wrong module may sometimes end up being called.
Whether this happens depends on the order of the calls in the BPF
program (as that affects whether sort() reorders the array of BTF
objects), making it especially hard to track down. Simon, credited as
reporter below, spent significant effort analysing and creating a
reproducer for this issue. The reproducer is added as a selftest in a
subsequent patch.
The fix is straight forward: simply don't use the stored pointer after
calling sort(). Since we already have an on-stack pointer to the BTF
object itself at the point where the function return, just use that, and
populate it from the cache entry in the branch where the lookup
succeeds.
Fixes: 2357672c54c3 ("bpf: Introduce BPF support for kernel module function calls")
Reported-by: Simon Sundberg <simon.sundberg@kau.se>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
kernel/bpf/verifier.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 633fd6da40c2460094b82ca7c2b9305eade05cf6..bf9996ea34fe1d80cef1c1ff3bbdb1030a976710 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2750,10 +2750,16 @@ static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
b->module = mod;
b->offset = offset;
+ /* sort() reorders entries by value, so b may no longer point
+ * to the right entry after this
+ */
sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
kfunc_btf_cmp_by_off, NULL);
+ } else {
+ btf = b->btf;
}
- return b->btf;
+
+ return btf;
}
void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab)
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf v2 2/3] selftests/bpf: Provide a generic [un]load_module helper
2024-10-10 13:27 [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier Toke Høiland-Jørgensen
2024-10-10 13:27 ` [PATCH bpf v2 1/3] bpf: fix kfunc btf caching for modules Toke Høiland-Jørgensen
@ 2024-10-10 13:27 ` Toke Høiland-Jørgensen
2024-10-10 13:27 ` [PATCH bpf v2 3/3] selftests/bpf: Add test for kfunc module order Toke Høiland-Jørgensen
2024-10-10 18:00 ` [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2024-10-10 13:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Kumar Kartikeya Dwivedi
Cc: Simon Sundberg, bpf, netdev, Toke Høiland-Jørgensen
From: Simon Sundberg <simon.sundberg@kau.se>
Generalize the previous [un]load_bpf_testmod() helpers (in
testing_helpers.c) to the more generic [un]load_module(), which can
load an arbitrary kernel module by name. This allows future selftests
to more easily load custom kernel modules other than bpf_testmod.ko.
Refactor [un]load_bpf_testmod() to wrap this new helper.
Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/testing_helpers.c | 34 +++++++++++++++++----------
tools/testing/selftests/bpf/testing_helpers.h | 2 ++
2 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index d3c3c3a24150f99abd13ecb7d7b11d8f7351560d..5e9f16683be5460b1a295fb9754df761cbd090ea 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -367,7 +367,7 @@ int delete_module(const char *name, int flags)
return syscall(__NR_delete_module, name, flags);
}
-int unload_bpf_testmod(bool verbose)
+int unload_module(const char *name, bool verbose)
{
int ret, cnt = 0;
@@ -375,11 +375,11 @@ int unload_bpf_testmod(bool verbose)
fprintf(stdout, "Failed to trigger kernel-side RCU sync!\n");
for (;;) {
- ret = delete_module("bpf_testmod", 0);
+ ret = delete_module(name, 0);
if (!ret || errno != EAGAIN)
break;
if (++cnt > 10000) {
- fprintf(stdout, "Unload of bpf_testmod timed out\n");
+ fprintf(stdout, "Unload of %s timed out\n", name);
break;
}
usleep(100);
@@ -388,41 +388,51 @@ int unload_bpf_testmod(bool verbose)
if (ret) {
if (errno == ENOENT) {
if (verbose)
- fprintf(stdout, "bpf_testmod.ko is already unloaded.\n");
+ fprintf(stdout, "%s.ko is already unloaded.\n", name);
return -1;
}
- fprintf(stdout, "Failed to unload bpf_testmod.ko from kernel: %d\n", -errno);
+ fprintf(stdout, "Failed to unload %s.ko from kernel: %d\n", name, -errno);
return -1;
}
if (verbose)
- fprintf(stdout, "Successfully unloaded bpf_testmod.ko.\n");
+ fprintf(stdout, "Successfully unloaded %s.ko.\n", name);
return 0;
}
-int load_bpf_testmod(bool verbose)
+int load_module(const char *path, bool verbose)
{
int fd;
if (verbose)
- fprintf(stdout, "Loading bpf_testmod.ko...\n");
+ fprintf(stdout, "Loading %s...\n", path);
- fd = open("bpf_testmod.ko", O_RDONLY);
+ fd = open(path, O_RDONLY);
if (fd < 0) {
- fprintf(stdout, "Can't find bpf_testmod.ko kernel module: %d\n", -errno);
+ fprintf(stdout, "Can't find %s kernel module: %d\n", path, -errno);
return -ENOENT;
}
if (finit_module(fd, "", 0)) {
- fprintf(stdout, "Failed to load bpf_testmod.ko into the kernel: %d\n", -errno);
+ fprintf(stdout, "Failed to load %s into the kernel: %d\n", path, -errno);
close(fd);
return -EINVAL;
}
close(fd);
if (verbose)
- fprintf(stdout, "Successfully loaded bpf_testmod.ko.\n");
+ fprintf(stdout, "Successfully loaded %s.\n", path);
return 0;
}
+int unload_bpf_testmod(bool verbose)
+{
+ return unload_module("bpf_testmod", verbose);
+}
+
+int load_bpf_testmod(bool verbose)
+{
+ return load_module("bpf_testmod.ko", verbose);
+}
+
/*
* Trigger synchronize_rcu() in kernel.
*/
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index d55f6ab124338ccab33bc120ca7e3baa18264aea..46d7f7089f636b0d2476859fd0fa5e1c4b305419 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -38,6 +38,8 @@ int unload_bpf_testmod(bool verbose);
int kern_sync_rcu(void);
int finit_module(int fd, const char *param_values, int flags);
int delete_module(const char *name, int flags);
+int load_module(const char *path, bool verbose);
+int unload_module(const char *name, bool verbose);
static inline __u64 get_time_ns(void)
{
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf v2 3/3] selftests/bpf: Add test for kfunc module order
2024-10-10 13:27 [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier Toke Høiland-Jørgensen
2024-10-10 13:27 ` [PATCH bpf v2 1/3] bpf: fix kfunc btf caching for modules Toke Høiland-Jørgensen
2024-10-10 13:27 ` [PATCH bpf v2 2/3] selftests/bpf: Provide a generic [un]load_module helper Toke Høiland-Jørgensen
@ 2024-10-10 13:27 ` Toke Høiland-Jørgensen
2024-10-10 18:00 ` [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Toke Høiland-Jørgensen @ 2024-10-10 13:27 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Kumar Kartikeya Dwivedi
Cc: Simon Sundberg, bpf, netdev, Toke Høiland-Jørgensen
From: Simon Sundberg <simon.sundberg@kau.se>
Add a test case for kfuncs from multiple external modules, checking
that the correct kfuncs are called regardless of which order they're
called in. Specifically, check that calling the kfuncs in an order
different from the one the modules' BTF are loaded in works.
Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
tools/testing/selftests/bpf/Makefile | 20 +++++++-
.../selftests/bpf/bpf_test_modorder_x/Makefile | 19 ++++++++
.../bpf/bpf_test_modorder_x/bpf_test_modorder_x.c | 39 +++++++++++++++
.../selftests/bpf/bpf_test_modorder_y/Makefile | 19 ++++++++
.../bpf/bpf_test_modorder_y/bpf_test_modorder_y.c | 39 +++++++++++++++
.../selftests/bpf/prog_tests/kfunc_module_order.c | 55 ++++++++++++++++++++++
.../selftests/bpf/progs/kfunc_module_order.c | 30 ++++++++++++
7 files changed, 220 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index f04af11df8eb5a1cecd75a4864d45c669433df61..6f9838a5dcc9a0a0e7cc249a4f93c87befafb699 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -157,7 +157,8 @@ TEST_GEN_PROGS_EXTENDED = \
flow_dissector_load test_flow_dissector test_tcp_check_syncookie_user \
test_lirc_mode2_user xdping test_cpp runqslower bench bpf_testmod.ko \
xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
- xdp_features bpf_test_no_cfi.ko
+ xdp_features bpf_test_no_cfi.ko bpf_test_modorder_x.ko \
+ bpf_test_modorder_y.ko
TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
@@ -303,6 +304,19 @@ $(OUTPUT)/bpf_test_no_cfi.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_te
$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_test_no_cfi
$(Q)cp bpf_test_no_cfi/bpf_test_no_cfi.ko $@
+$(OUTPUT)/bpf_test_modorder_x.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_test_modorder_x/Makefile bpf_test_modorder_x/*.[ch])
+ $(call msg,MOD,,$@)
+ $(Q)$(RM) bpf_test_modorder_x/bpf_test_modorder_x.ko # force re-compilation
+ $(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_test_modorder_x
+ $(Q)cp bpf_test_modorder_x/bpf_test_modorder_x.ko $@
+
+$(OUTPUT)/bpf_test_modorder_y.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_test_modorder_y/Makefile bpf_test_modorder_y/*.[ch])
+ $(call msg,MOD,,$@)
+ $(Q)$(RM) bpf_test_modorder_y/bpf_test_modorder_y.ko # force re-compilation
+ $(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_test_modorder_y
+ $(Q)cp bpf_test_modorder_y/bpf_test_modorder_y.ko $@
+
+
DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
ifneq ($(CROSS_COMPILE),)
CROSS_BPFTOOL := $(SCRATCH_DIR)/sbin/bpftool
@@ -722,6 +736,8 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \
ip_check_defrag_frags.h
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
$(OUTPUT)/bpf_test_no_cfi.ko \
+ $(OUTPUT)/bpf_test_modorder_x.ko \
+ $(OUTPUT)/bpf_test_modorder_y.ko \
$(OUTPUT)/liburandom_read.so \
$(OUTPUT)/xdp_synproxy \
$(OUTPUT)/sign-file \
@@ -856,6 +872,8 @@ EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
$(addprefix $(OUTPUT)/,*.o *.d *.skel.h *.lskel.h *.subskel.h \
no_alu32 cpuv4 bpf_gcc bpf_testmod.ko \
bpf_test_no_cfi.ko \
+ bpf_test_modorder_x.ko \
+ bpf_test_modorder_y.ko \
liburandom_read.so) \
$(OUTPUT)/FEATURE-DUMP.selftests
diff --git a/tools/testing/selftests/bpf/bpf_test_modorder_x/Makefile b/tools/testing/selftests/bpf/bpf_test_modorder_x/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..40b25b98ad1b622c6a5c3c00d0625595349bb677
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_modorder_x/Makefile
@@ -0,0 +1,19 @@
+BPF_TESTMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(BPF_TESTMOD_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = bpf_test_modorder_x.ko
+
+obj-m += bpf_test_modorder_x.o
+
+all:
+ +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) modules
+
+clean:
+ +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) clean
+
diff --git a/tools/testing/selftests/bpf/bpf_test_modorder_x/bpf_test_modorder_x.c b/tools/testing/selftests/bpf/bpf_test_modorder_x/bpf_test_modorder_x.c
new file mode 100644
index 0000000000000000000000000000000000000000..0cc747fa912fcd5b6738af15dc1b8dfb88c33f6b
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_modorder_x/bpf_test_modorder_x.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/module.h>
+#include <linux/init.h>
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_test_modorder_retx(void)
+{
+ return 'x';
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(bpf_test_modorder_kfunc_x_ids)
+BTF_ID_FLAGS(func, bpf_test_modorder_retx);
+BTF_KFUNCS_END(bpf_test_modorder_kfunc_x_ids)
+
+static const struct btf_kfunc_id_set bpf_test_modorder_x_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_test_modorder_kfunc_x_ids,
+};
+
+static int __init bpf_test_modorder_x_init(void)
+{
+ return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+ &bpf_test_modorder_x_set);
+}
+
+static void __exit bpf_test_modorder_x_exit(void)
+{
+}
+
+module_init(bpf_test_modorder_x_init);
+module_exit(bpf_test_modorder_x_exit);
+
+MODULE_DESCRIPTION("BPF selftest ordertest module X");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/bpf/bpf_test_modorder_y/Makefile b/tools/testing/selftests/bpf/bpf_test_modorder_y/Makefile
new file mode 100644
index 0000000000000000000000000000000000000000..52c3ab9d84e29c794f57c1f75be03b46d80d4a06
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_modorder_y/Makefile
@@ -0,0 +1,19 @@
+BPF_TESTMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(BPF_TESTMOD_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = bpf_test_modorder_y.ko
+
+obj-m += bpf_test_modorder_y.o
+
+all:
+ +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) modules
+
+clean:
+ +$(Q)make -C $(KDIR) M=$(BPF_TESTMOD_DIR) clean
+
diff --git a/tools/testing/selftests/bpf/bpf_test_modorder_y/bpf_test_modorder_y.c b/tools/testing/selftests/bpf/bpf_test_modorder_y/bpf_test_modorder_y.c
new file mode 100644
index 0000000000000000000000000000000000000000..c627ee085d1305af98c5d7f66d99dcfbf98dc4e1
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_modorder_y/bpf_test_modorder_y.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/module.h>
+#include <linux/init.h>
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc int bpf_test_modorder_rety(void)
+{
+ return 'y';
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(bpf_test_modorder_kfunc_y_ids)
+BTF_ID_FLAGS(func, bpf_test_modorder_rety);
+BTF_KFUNCS_END(bpf_test_modorder_kfunc_y_ids)
+
+static const struct btf_kfunc_id_set bpf_test_modorder_y_set = {
+ .owner = THIS_MODULE,
+ .set = &bpf_test_modorder_kfunc_y_ids,
+};
+
+static int __init bpf_test_modorder_y_init(void)
+{
+ return register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS,
+ &bpf_test_modorder_y_set);
+}
+
+static void __exit bpf_test_modorder_y_exit(void)
+{
+}
+
+module_init(bpf_test_modorder_y_init);
+module_exit(bpf_test_modorder_y_exit);
+
+MODULE_DESCRIPTION("BPF selftest ordertest module Y");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c b/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c
new file mode 100644
index 0000000000000000000000000000000000000000..48c0560d398e2a14d0682e309cdfc99def244720
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_module_order.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <testing_helpers.h>
+
+#include "kfunc_module_order.skel.h"
+
+static int test_run_prog(const struct bpf_program *prog,
+ struct bpf_test_run_opts *opts)
+{
+ int err;
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(prog), opts);
+ if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
+ return err;
+
+ if (!ASSERT_EQ((int)opts->retval, 0, bpf_program__name(prog)))
+ return -EINVAL;
+
+ return 0;
+}
+
+void test_kfunc_module_order(void)
+{
+ struct kfunc_module_order *skel;
+ char pkt_data[64] = {};
+ int err = 0;
+
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, test_opts, .data_in = pkt_data,
+ .data_size_in = sizeof(pkt_data));
+
+ err = load_module("bpf_test_modorder_x.ko",
+ env_verbosity > VERBOSE_NONE);
+ if (!ASSERT_OK(err, "load bpf_test_modorder_x.ko"))
+ return;
+
+ err = load_module("bpf_test_modorder_y.ko",
+ env_verbosity > VERBOSE_NONE);
+ if (!ASSERT_OK(err, "load bpf_test_modorder_y.ko"))
+ goto exit_modx;
+
+ skel = kfunc_module_order__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "kfunc_module_order__open_and_load()")) {
+ err = -EINVAL;
+ goto exit_mods;
+ }
+
+ test_run_prog(skel->progs.call_kfunc_xy, &test_opts);
+ test_run_prog(skel->progs.call_kfunc_yx, &test_opts);
+
+ kfunc_module_order__destroy(skel);
+exit_mods:
+ unload_module("bpf_test_modorder_y", env_verbosity > VERBOSE_NONE);
+exit_modx:
+ unload_module("bpf_test_modorder_x", env_verbosity > VERBOSE_NONE);
+}
diff --git a/tools/testing/selftests/bpf/progs/kfunc_module_order.c b/tools/testing/selftests/bpf/progs/kfunc_module_order.c
new file mode 100644
index 0000000000000000000000000000000000000000..76003d04c95f4eaef4c9f3ec640a0da2a20253e2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kfunc_module_order.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+extern int bpf_test_modorder_retx(void) __ksym;
+extern int bpf_test_modorder_rety(void) __ksym;
+
+SEC("classifier")
+int call_kfunc_xy(struct __sk_buff *skb)
+{
+ int ret1, ret2;
+
+ ret1 = bpf_test_modorder_retx();
+ ret2 = bpf_test_modorder_rety();
+
+ return ret1 == 'x' && ret2 == 'y' ? 0 : -1;
+}
+
+SEC("classifier")
+int call_kfunc_yx(struct __sk_buff *skb)
+{
+ int ret1, ret2;
+
+ ret1 = bpf_test_modorder_rety();
+ ret2 = bpf_test_modorder_retx();
+
+ return ret1 == 'y' && ret2 == 'x' ? 0 : -1;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.47.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier
2024-10-10 13:27 [PATCH bpf v2 0/3] Fix caching of BTF for kfuncs in the verifier Toke Høiland-Jørgensen
` (2 preceding siblings ...)
2024-10-10 13:27 ` [PATCH bpf v2 3/3] selftests/bpf: Add test for kfunc module order Toke Høiland-Jørgensen
@ 2024-10-10 18:00 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-10-10 18:00 UTC (permalink / raw)
To: =?utf-8?b?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIDx0b2tlQHJlZGhhdC5jb20+?=
Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song,
yonghong.song, kpsingh, sdf, haoluo, jolsa, memxor,
simon.sundberg, bpf, netdev
Hello:
This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Thu, 10 Oct 2024 15:27:06 +0200 you wrote:
> When playing around with defining kfuncs in some custom modules, we
> noticed that if a BPF program calls two functions with the same
> signature in two different modules, the function from the wrong module
> may sometimes end up being called. Whether this happens depends on the
> order of the calls in the BPF program, which turns out to be due to the
> use of sort() inside __find_kfunc_desc_btf() in the verifier code.
>
> [...]
Here is the summary with links:
- [bpf,v2,1/3] bpf: fix kfunc btf caching for modules
https://git.kernel.org/bpf/bpf/c/6cb86a0fdece
- [bpf,v2,2/3] selftests/bpf: Provide a generic [un]load_module helper
https://git.kernel.org/bpf/bpf/c/4192bb294f80
- [bpf,v2,3/3] selftests/bpf: Add test for kfunc module order
https://git.kernel.org/bpf/bpf/c/f91b256644ea
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread