* [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type.
@ 2024-02-16 2:03 thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 1/2] bpf: " thinker.li
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: thinker.li @ 2024-02-16 2:03 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
Recently, cfi_stubs were introduced. However, existing struct_ops
types that are not in the upstream may not be aware of this, resulting
in kernel crashes. By rejecting struct_ops types that do not provide
cfi_stubs properly during registration, these crashes can be avoided.
---
Changes from v1:
- Check *(void **)(cfi_stubs + moff) to make sure stub functions are
provided for every operator.
- Add a test case to ensure that struct_ops rejects incomplete
cfi_stub.
v1: https://lore.kernel.org/all/20240215022401.1882010-1-thinker.li@gmail.com/
Kui-Feng Lee (2):
bpf: Check cfi_stubs before registering a struct_ops type.
selftests/bpf: Test case for lacking CFI stub functions.
kernel/bpf/bpf_struct_ops.c | 14 +++
tools/testing/selftests/bpf/Makefile | 10 +-
.../selftests/bpf/bpf_test_no_cfi/Makefile | 19 ++++
.../bpf/bpf_test_no_cfi/bpf_test_no_cfi.c | 93 +++++++++++++++++++
.../bpf/prog_tests/test_struct_ops_no_cfi.c | 31 +++++++
tools/testing/selftests/bpf/testing_helpers.c | 4 +-
tools/testing/selftests/bpf/testing_helpers.h | 2 +
7 files changed, 170 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH bpf-next v2 1/2] bpf: Check cfi_stubs before registering a struct_ops type.
2024-02-16 2:03 [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type thinker.li
@ 2024-02-16 2:03 ` thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test case for lacking CFI stub functions thinker.li
2024-02-16 18:40 ` [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type Kui-Feng Lee
2 siblings, 0 replies; 5+ messages in thread
From: thinker.li @ 2024-02-16 2:03 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
Recently, cfi_stubs were introduced. However, existing struct_ops types
that are not in the upstream may not be aware of this, resulting in kernel
crashes. By rejecting struct_ops types that do not provide cfi_stubs during
registration, these crashes can be avoided.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
kernel/bpf/bpf_struct_ops.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 0d7be97a2411..9febd450d224 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -302,6 +302,11 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
}
sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
+ if (!st_ops->cfi_stubs) {
+ pr_warn("struct %s has no cfi_stubs\n", st_ops->name);
+ return -EINVAL;
+ }
+
type_id = btf_find_by_name_kind(btf, st_ops->name,
BTF_KIND_STRUCT);
if (type_id < 0) {
@@ -339,6 +344,7 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
for_each_member(i, t, member) {
const struct btf_type *func_proto;
+ u32 moff;
mname = btf_name_by_offset(btf, member->name_off);
if (!*mname) {
@@ -361,6 +367,14 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc,
if (!func_proto)
continue;
+ moff = __btf_member_bit_offset(t, member) / 8;
+ if (!*(void **)(st_ops->cfi_stubs + moff)) {
+ pr_warn("member %s in struct %s has no cfi stub function\n",
+ mname, st_ops->name);
+ err = -EINVAL;
+ goto errout;
+ }
+
if (btf_distill_func_proto(log, btf,
func_proto, mname,
&st_ops->func_models[i])) {
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Test case for lacking CFI stub functions.
2024-02-16 2:03 [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 1/2] bpf: " thinker.li
@ 2024-02-16 2:03 ` thinker.li
2024-02-16 18:40 ` [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type Kui-Feng Lee
2 siblings, 0 replies; 5+ messages in thread
From: thinker.li @ 2024-02-16 2:03 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
Ensure struct_ops rejects the registration of struct_ops types without
proper CFI stub functions.
bpf_test_no_cfi.ko is a module that attempts to register a struct_ops type
called "bpf_test_no_cfi_ops" with varying levels of cfi_stubs. It starts
with a NULL cfi_stub and ends with a fully complete cfi_stub. Only the
fully complete cfi_stub should be accepted by struct_ops. The module can
only be loaded successfully if these registrations yield the expected
results.
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
tools/testing/selftests/bpf/Makefile | 10 +-
.../selftests/bpf/bpf_test_no_cfi/Makefile | 19 ++++
.../bpf/bpf_test_no_cfi/bpf_test_no_cfi.c | 93 +++++++++++++++++++
.../bpf/prog_tests/test_struct_ops_no_cfi.c | 31 +++++++
tools/testing/selftests/bpf/testing_helpers.c | 4 +-
tools/testing/selftests/bpf/testing_helpers.h | 2 +
6 files changed, 156 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index a38a3001527c..2738d85680ac 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -132,7 +132,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
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
+ xdp_features bpf_test_no_cfi.ko
TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
@@ -255,6 +255,12 @@ $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_testmo
$(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_testmod
$(Q)cp bpf_testmod/bpf_testmod.ko $@
+$(OUTPUT)/bpf_test_no_cfi.ko: $(VMLINUX_BTF) $(RESOLVE_BTFIDS) $(wildcard bpf_test_no_cfi/Makefile bpf_test_no_cfi/*.[ch])
+ $(call msg,MOD,,$@)
+ $(Q)$(RM) bpf_test_no_cfi/bpf_test_no_cfi.ko # force re-compilation
+ $(Q)$(MAKE) $(submake_extras) RESOLVE_BTFIDS=$(RESOLVE_BTFIDS) -C bpf_test_no_cfi
+ $(Q)cp bpf_test_no_cfi/bpf_test_no_cfi.ko $@
+
DEFAULT_BPFTOOL := $(HOST_SCRATCH_DIR)/sbin/bpftool
ifneq ($(CROSS_COMPILE),)
CROSS_BPFTOOL := $(SCRATCH_DIR)/sbin/bpftool
@@ -629,6 +635,7 @@ TRUNNER_EXTRA_SOURCES := test_progs.c \
flow_dissector_load.h \
ip_check_defrag_frags.h
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \
+ $(OUTPUT)/bpf_test_no_cfi.ko \
$(OUTPUT)/liburandom_read.so \
$(OUTPUT)/xdp_synproxy \
$(OUTPUT)/sign-file \
@@ -757,6 +764,7 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
feature bpftool \
$(addprefix $(OUTPUT)/,*.o *.skel.h *.lskel.h *.subskel.h \
no_alu32 cpuv4 bpf_gcc bpf_testmod.ko \
+ bpf_test_no_cfi.ko \
liburandom_read.so)
.PHONY: docs docs-clean
diff --git a/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile b/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
new file mode 100644
index 000000000000..ed5143b79edf
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
@@ -0,0 +1,19 @@
+BPF_TEST_NO_CFI_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
+KDIR ?= $(abspath $(BPF_TEST_NO_CFI_DIR)/../../../../..)
+
+ifeq ($(V),1)
+Q =
+else
+Q = @
+endif
+
+MODULES = bpf_test_no_cfi.ko
+
+obj-m += bpf_test_no_cfi.o
+
+all:
+ +$(Q)make -C $(KDIR) M=$(BPF_TEST_NO_CFI_DIR) modules
+
+clean:
+ +$(Q)make -C $(KDIR) M=$(BPF_TEST_NO_CFI_DIR) clean
+
diff --git a/tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c b/tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
new file mode 100644
index 000000000000..0fb63feecb31
--- /dev/null
+++ b/tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+#include <linux/bpf.h>
+#include <linux/btf.h>
+#include <linux/init.h>
+#include <linux/module.h>
+
+struct bpf_test_no_cfi_ops {
+ void (*fn_1)(void);
+ void (*fn_2)(void);
+};
+
+static int dummy_init(struct btf *btf)
+{
+ return 0;
+}
+
+static int dummy_init_member(const struct btf_type *t,
+ const struct btf_member *member,
+ void *kdata, const void *udata)
+{
+ return 0;
+}
+
+static int dummy_reg(void *kdata)
+{
+ return 0;
+}
+
+static void dummy_unreg(void *kdata)
+{
+}
+
+static const struct bpf_verifier_ops dummy_verifier_ops;
+
+static void bpf_test_no_cfi_ops__fn_1(void)
+{
+}
+
+static void bpf_test_no_cfi_ops__fn_2(void)
+{
+}
+
+static struct bpf_test_no_cfi_ops __bpf_test_no_cfi_ops;
+
+static struct bpf_struct_ops bpf_bpf_test_no_cif_ops = {
+ .verifier_ops = &dummy_verifier_ops,
+ .init = dummy_init,
+ .init_member = dummy_init_member,
+ .reg = dummy_reg,
+ .unreg = dummy_unreg,
+ .name = "bpf_test_no_cfi_ops",
+ .owner = THIS_MODULE,
+};
+
+static int bpf_test_no_cfi_init(void)
+{
+ int ret;
+
+ ret = register_bpf_struct_ops(&bpf_bpf_test_no_cif_ops,
+ bpf_test_no_cfi_ops);
+ if (!ret)
+ return -EINVAL;
+
+ bpf_bpf_test_no_cif_ops.cfi_stubs = &__bpf_test_no_cfi_ops;
+ ret = register_bpf_struct_ops(&bpf_bpf_test_no_cif_ops,
+ bpf_test_no_cfi_ops);
+ if (!ret)
+ return -EINVAL;
+
+ __bpf_test_no_cfi_ops.fn_1 = bpf_test_no_cfi_ops__fn_1;
+ ret = register_bpf_struct_ops(&bpf_bpf_test_no_cif_ops,
+ bpf_test_no_cfi_ops);
+ if (!ret)
+ return -EINVAL;
+
+ __bpf_test_no_cfi_ops.fn_2 = bpf_test_no_cfi_ops__fn_2;
+ ret = register_bpf_struct_ops(&bpf_bpf_test_no_cif_ops,
+ bpf_test_no_cfi_ops);
+ return ret;
+}
+
+static void bpf_test_no_cfi_exit(void)
+{
+}
+
+module_init(bpf_test_no_cfi_init);
+module_exit(bpf_test_no_cfi_exit);
+
+MODULE_AUTHOR("Kuifeng Lee");
+MODULE_DESCRIPTION("BPF no cfi_stubs test module");
+MODULE_LICENSE("Dual BSD/GPL");
+
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
new file mode 100644
index 000000000000..11fbd9ebf1f8
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <testing_helpers.h>
+
+void test_struct_ops_no_cfi(void)
+{
+ int fd;
+ int err;
+
+ fd = open("bpf_test_no_cfi.ko", O_RDONLY);
+ if (!ASSERT_GT(fd, 0, "open")) {
+ close(fd);
+ return;
+ }
+
+ /* The module will try to register a struct_ops type with
+ * no cfi_stubs, incomplete cfi_stubs, and full cfi_stubs.
+ *
+ * Only full cfi_stubs should be allowed. The module will be loaded
+ * successfully if the result of the registration is as expected,
+ * or it fails.
+ */
+ err = finit_module(fd, "", 0);
+ close(fd);
+ if (!ASSERT_OK(err, "finit_module"))
+ return;
+
+ err = delete_module("bpf_test_no_cfi", 0);
+ ASSERT_OK(err, "delete_module");
+}
diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c
index a59e56d804ee..28b6646662af 100644
--- a/tools/testing/selftests/bpf/testing_helpers.c
+++ b/tools/testing/selftests/bpf/testing_helpers.c
@@ -356,12 +356,12 @@ __u64 read_perf_max_sample_freq(void)
return sample_freq;
}
-static int finit_module(int fd, const char *param_values, int flags)
+int finit_module(int fd, const char *param_values, int flags)
{
return syscall(__NR_finit_module, fd, param_values, flags);
}
-static int delete_module(const char *name, int flags)
+int delete_module(const char *name, int flags)
{
return syscall(__NR_delete_module, name, flags);
}
diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h
index d14de81727e6..d55f6ab12433 100644
--- a/tools/testing/selftests/bpf/testing_helpers.h
+++ b/tools/testing/selftests/bpf/testing_helpers.h
@@ -36,6 +36,8 @@ __u64 read_perf_max_sample_freq(void);
int load_bpf_testmod(bool verbose);
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);
static inline __u64 get_time_ns(void)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type.
2024-02-16 2:03 [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 1/2] bpf: " thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test case for lacking CFI stub functions thinker.li
@ 2024-02-16 18:40 ` Kui-Feng Lee
2024-02-16 19:18 ` Kui-Feng Lee
2 siblings, 1 reply; 5+ messages in thread
From: Kui-Feng Lee @ 2024-02-16 18:40 UTC (permalink / raw)
To: thinker.li, bpf, ast, martin.lau, song, kernel-team, andrii; +Cc: kuifeng
It fails on CI.
I am fixing it.
On 2/15/24 18:03, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
>
> Recently, cfi_stubs were introduced. However, existing struct_ops
> types that are not in the upstream may not be aware of this, resulting
> in kernel crashes. By rejecting struct_ops types that do not provide
> cfi_stubs properly during registration, these crashes can be avoided.
>
> ---
> Changes from v1:
>
> - Check *(void **)(cfi_stubs + moff) to make sure stub functions are
> provided for every operator.
>
> - Add a test case to ensure that struct_ops rejects incomplete
> cfi_stub.
>
> v1: https://lore.kernel.org/all/20240215022401.1882010-1-thinker.li@gmail.com/
>
> Kui-Feng Lee (2):
> bpf: Check cfi_stubs before registering a struct_ops type.
> selftests/bpf: Test case for lacking CFI stub functions.
>
> kernel/bpf/bpf_struct_ops.c | 14 +++
> tools/testing/selftests/bpf/Makefile | 10 +-
> .../selftests/bpf/bpf_test_no_cfi/Makefile | 19 ++++
> .../bpf/bpf_test_no_cfi/bpf_test_no_cfi.c | 93 +++++++++++++++++++
> .../bpf/prog_tests/test_struct_ops_no_cfi.c | 31 +++++++
> tools/testing/selftests/bpf/testing_helpers.c | 4 +-
> tools/testing/selftests/bpf/testing_helpers.h | 2 +
> 7 files changed, 170 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
> create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
> create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type.
2024-02-16 18:40 ` [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type Kui-Feng Lee
@ 2024-02-16 19:18 ` Kui-Feng Lee
0 siblings, 0 replies; 5+ messages in thread
From: Kui-Feng Lee @ 2024-02-16 19:18 UTC (permalink / raw)
To: thinker.li, bpf, ast, martin.lau, song, kernel-team, andrii; +Cc: kuifeng
Somehow, bpf_tcp_ca misses a stub function.
Will add it in the next version.
On 2/16/24 10:40, Kui-Feng Lee wrote:
> It fails on CI.
> I am fixing it.
>
> On 2/15/24 18:03, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> Recently, cfi_stubs were introduced. However, existing struct_ops
>> types that are not in the upstream may not be aware of this, resulting
>> in kernel crashes. By rejecting struct_ops types that do not provide
>> cfi_stubs properly during registration, these crashes can be avoided.
>>
>> ---
>> Changes from v1:
>>
>> - Check *(void **)(cfi_stubs + moff) to make sure stub functions are
>> provided for every operator.
>>
>> - Add a test case to ensure that struct_ops rejects incomplete
>> cfi_stub.
>>
>> v1:
>> https://lore.kernel.org/all/20240215022401.1882010-1-thinker.li@gmail.com/
>>
>> Kui-Feng Lee (2):
>> bpf: Check cfi_stubs before registering a struct_ops type.
>> selftests/bpf: Test case for lacking CFI stub functions.
>>
>> kernel/bpf/bpf_struct_ops.c | 14 +++
>> tools/testing/selftests/bpf/Makefile | 10 +-
>> .../selftests/bpf/bpf_test_no_cfi/Makefile | 19 ++++
>> .../bpf/bpf_test_no_cfi/bpf_test_no_cfi.c | 93 +++++++++++++++++++
>> .../bpf/prog_tests/test_struct_ops_no_cfi.c | 31 +++++++
>> tools/testing/selftests/bpf/testing_helpers.c | 4 +-
>> tools/testing/selftests/bpf/testing_helpers.h | 2 +
>> 7 files changed, 170 insertions(+), 3 deletions(-)
>> create mode 100644 tools/testing/selftests/bpf/bpf_test_no_cfi/Makefile
>> create mode 100644
>> tools/testing/selftests/bpf/bpf_test_no_cfi/bpf_test_no_cfi.c
>> create mode 100644
>> tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-16 19:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-16 2:03 [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 1/2] bpf: " thinker.li
2024-02-16 2:03 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test case for lacking CFI stub functions thinker.li
2024-02-16 18:40 ` [PATCH bpf-next v2 0/2] Check cfi_stubs before registering a struct_ops type Kui-Feng Lee
2024-02-16 19:18 ` Kui-Feng Lee
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.