* [PATCH bpf-next v4 0/3] Check cfi_stubs before registering a struct_ops type.
@ 2024-02-21 7:52 thinker.li
2024-02-21 7:52 ` [PATCH bpf-next v4 1/3] bpf, net: allow passing NULL prog to check_member thinker.li
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: thinker.li @ 2024-02-21 7:52 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 v3:
- Remove CFI stub function for get_info.
- Allow passing NULL prog arg to check_member of struct
bpf_struct_ops type.
- Call check_member to determines if a CFI stub function should be
defined for an operator.
Changes from v2:
- Add a stub function for get_info of struct tcp_congestion_ops.
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.
v3: https://lore.kernel.org/all/20240216193434.735874-1-thinker.li@gmail.com/
v2: https://lore.kernel.org/all/20240216020350.2061373-1-thinker.li@gmail.com/
v1: https://lore.kernel.org/all/20240215022401.1882010-1-thinker.li@gmail.com/
Kui-Feng Lee (3):
bpf, net: allow passing NULL prog to check_member.
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 | 17 ++++
net/bpf/bpf_dummy_struct_ops.c | 2 +-
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 | 38 ++++++++
tools/testing/selftests/bpf/testing_helpers.c | 4 +-
tools/testing/selftests/bpf/testing_helpers.h | 2 +
8 files changed, 181 insertions(+), 4 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] 7+ messages in thread* [PATCH bpf-next v4 1/3] bpf, net: allow passing NULL prog to check_member. 2024-02-21 7:52 [PATCH bpf-next v4 0/3] Check cfi_stubs before registering a struct_ops type thinker.li @ 2024-02-21 7:52 ` thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 3/3] selftests/bpf: Test case for lacking CFI stub functions thinker.li 2 siblings, 0 replies; 7+ messages in thread From: thinker.li @ 2024-02-21 7:52 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> To reuse the check_member function of the bpf_struct_ops structure for checking if an operator is supported, we permit passing a NULL value for the "prog" argument in check_member. The check_member function of the bpf_struct_ops structure will be utilized for checking cfi_stubs in a subsequent patch when registering a struct_ops type. netdev@vger.kernel.org Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com> --- net/bpf/bpf_dummy_struct_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c index 02de71719aed..5fe5461d3173 100644 --- a/net/bpf/bpf_dummy_struct_ops.c +++ b/net/bpf/bpf_dummy_struct_ops.c @@ -178,7 +178,7 @@ static int bpf_dummy_ops_check_member(const struct btf_type *t, case offsetof(struct bpf_dummy_ops, test_sleepable): break; default: - if (prog->aux->sleepable) + if (prog && prog->aux->sleepable) return -EINVAL; } -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type. 2024-02-21 7:52 [PATCH bpf-next v4 0/3] Check cfi_stubs before registering a struct_ops type thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 1/3] bpf, net: allow passing NULL prog to check_member thinker.li @ 2024-02-21 7:52 ` thinker.li 2024-02-21 18:25 ` Martin KaFai Lau 2024-02-21 7:52 ` [PATCH bpf-next v4 3/3] selftests/bpf: Test case for lacking CFI stub functions thinker.li 2 siblings, 1 reply; 7+ messages in thread From: thinker.li @ 2024-02-21 7:52 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 | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c index 0d7be97a2411..c1c502caae08 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,17 @@ 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; + err = st_ops->check_member ? + st_ops->check_member(t, member, NULL) : 0; + + if (!err && !*(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] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type. 2024-02-21 7:52 ` [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type thinker.li @ 2024-02-21 18:25 ` Martin KaFai Lau 2024-02-21 23:13 ` Kui-Feng Lee 0 siblings, 1 reply; 7+ messages in thread From: Martin KaFai Lau @ 2024-02-21 18:25 UTC (permalink / raw) To: thinker.li; +Cc: sinquersw, kuifeng, bpf, ast, song, kernel-team, andrii On 2/20/24 11:52 PM, 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 during > registration, these crashes can be avoided. > > Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com> > --- > kernel/bpf/bpf_struct_ops.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c > index 0d7be97a2411..c1c502caae08 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,17 @@ 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; > + err = st_ops->check_member ? > + st_ops->check_member(t, member, NULL) : 0; I don't think it is necessary to make check_member more complicated by taking NULL prog. The struct_ops implementer then needs to handle this extra NULL prog case. Have you thought about Alexei's earlier suggestion in v3 to reuse the NULL member in cfi_stubs to flag unsupported member and remove the unsupported_ops[] from bpf_tcp_ca.c? If the verifier can consistently reject loading unsupported bpf prog, it will not reach the bpf_struct_ops_map_update_elem and then hits the NULL member in cfi_stubs during map_update_elem. Untested code: diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 011d54a1dc53..c57cb0e2a8a7 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -20370,6 +20370,7 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env) u32 btf_id, member_idx; struct btf *btf; const char *mname; + u32 moff; if (!prog->gpl_compatible) { verbose(env, "struct ops programs must have a GPL compatible license\n"); @@ -20417,11 +20418,18 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env) return -EINVAL; } + moff = __btf_member_bit_offset(t, member) / 8; + if (!*(void **)(st_ops->cfi_stubs + moff)) { + verbose(env, "attach to unsupported member %s of struct %s\n", + mname, st_ops->name); + return -ENOTSUPP; + } + if (st_ops->check_member) { int err = st_ops->check_member(t, member, prog); if (err) { - verbose(env, "attach to unsupported member %s of struct %s\n", + verbose(env, "cannot attach to member %s of struct %s\n", mname, st_ops->name); return err; } diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c index 7f518ea5f4ac..bcb1fcd00973 100644 --- a/net/ipv4/bpf_tcp_ca.c +++ b/net/ipv4/bpf_tcp_ca.c @@ -14,10 +14,6 @@ /* "extern" is to avoid sparse warning. It is only used in bpf_struct_ops.c. */ static struct bpf_struct_ops bpf_tcp_congestion_ops; -static u32 unsupported_ops[] = { - offsetof(struct tcp_congestion_ops, get_info), -}; - static const struct btf_type *tcp_sock_type; static u32 tcp_sock_id, sock_id; static const struct btf_type *tcp_congestion_ops_type; @@ -45,18 +41,6 @@ static int bpf_tcp_ca_init(struct btf *btf) return 0; } -static bool is_unsupported(u32 member_offset) -{ - unsigned int i; - - for (i = 0; i < ARRAY_SIZE(unsupported_ops); i++) { - if (member_offset == unsupported_ops[i]) - return true; - } - - return false; -} - static bool bpf_tcp_ca_is_valid_access(int off, int size, enum bpf_access_type type, const struct bpf_prog *prog, @@ -248,15 +232,6 @@ static int bpf_tcp_ca_init_member(const struct btf_type *t, return 0; } -static int bpf_tcp_ca_check_member(const struct btf_type *t, - const struct btf_member *member, - const struct bpf_prog *prog) -{ - if (is_unsupported(__btf_member_bit_offset(t, member) / 8)) - return -ENOTSUPP; - return 0; -} - static int bpf_tcp_ca_reg(void *kdata) { return tcp_register_congestion_control(kdata); @@ -350,7 +325,6 @@ static struct bpf_struct_ops bpf_tcp_congestion_ops = { .reg = bpf_tcp_ca_reg, .unreg = bpf_tcp_ca_unreg, .update = bpf_tcp_ca_update, - .check_member = bpf_tcp_ca_check_member, .init_member = bpf_tcp_ca_init_member, .init = bpf_tcp_ca_init, .validate = bpf_tcp_ca_validate, -- 2.34.1 > + > + if (!err && !*(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])) { ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type. 2024-02-21 18:25 ` Martin KaFai Lau @ 2024-02-21 23:13 ` Kui-Feng Lee 2024-02-22 1:11 ` Kui-Feng Lee 0 siblings, 1 reply; 7+ messages in thread From: Kui-Feng Lee @ 2024-02-21 23:13 UTC (permalink / raw) To: Martin KaFai Lau, thinker.li; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii On 2/21/24 10:25, Martin KaFai Lau wrote: > On 2/20/24 11:52 PM, 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 >> during >> registration, these crashes can be avoided. >> >> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com> >> --- >> kernel/bpf/bpf_struct_ops.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c >> index 0d7be97a2411..c1c502caae08 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,17 @@ 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; >> + err = st_ops->check_member ? >> + st_ops->check_member(t, member, NULL) : 0; > > I don't think it is necessary to make check_member more complicated by > taking > NULL prog. The struct_ops implementer then needs to handle this extra NULL > prog case. > > Have you thought about Alexei's earlier suggestion in v3 to reuse the NULL > member in cfi_stubs to flag unsupported member and remove the > unsupported_ops[] > from bpf_tcp_ca.c? > > If the verifier can consistently reject loading unsupported bpf prog, it > will > not reach the bpf_struct_ops_map_update_elem and then hits the NULL member > in cfi_stubs during map_update_elem. > Ok! I misunderstood previously. I will go this way. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type. 2024-02-21 23:13 ` Kui-Feng Lee @ 2024-02-22 1:11 ` Kui-Feng Lee 0 siblings, 0 replies; 7+ messages in thread From: Kui-Feng Lee @ 2024-02-22 1:11 UTC (permalink / raw) To: Martin KaFai Lau, thinker.li; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii On 2/21/24 15:13, Kui-Feng Lee wrote: > > > On 2/21/24 10:25, Martin KaFai Lau wrote: >> On 2/20/24 11:52 PM, 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 >>> during >>> registration, these crashes can be avoided. >>> >>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com> >>> --- >>> kernel/bpf/bpf_struct_ops.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c >>> index 0d7be97a2411..c1c502caae08 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,17 @@ 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; >>> + err = st_ops->check_member ? >>> + st_ops->check_member(t, member, NULL) : 0; >> >> I don't think it is necessary to make check_member more complicated by >> taking >> NULL prog. The struct_ops implementer then needs to handle this extra >> NULL >> prog case. >> >> Have you thought about Alexei's earlier suggestion in v3 to reuse the >> NULL >> member in cfi_stubs to flag unsupported member and remove the >> unsupported_ops[] >> from bpf_tcp_ca.c? >> >> If the verifier can consistently reject loading unsupported bpf prog, >> it will >> not reach the bpf_struct_ops_map_update_elem and then hits the NULL >> member >> in cfi_stubs during map_update_elem. >> > > Ok! I misunderstood previously. I will go this way. > According to the off-line discussion, the changes for unsupported_ops[] should be in a separate patchset. The check of (void **)(st_ops->cfi_stubs + moff)) will be removed. Changes of check_member should be removed as well. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v4 3/3] selftests/bpf: Test case for lacking CFI stub functions. 2024-02-21 7:52 [PATCH bpf-next v4 0/3] Check cfi_stubs before registering a struct_ops type thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 1/3] bpf, net: allow passing NULL prog to check_member thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type thinker.li @ 2024-02-21 7:52 ` thinker.li 2 siblings, 0 replies; 7+ messages in thread From: thinker.li @ 2024-02-21 7:52 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 | 38 ++++++++ tools/testing/selftests/bpf/testing_helpers.c | 4 +- tools/testing/selftests/bpf/testing_helpers.h | 2 + 6 files changed, 163 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 dbb8c5f94f34..c219da5e60e6 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 @@ -254,6 +254,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 @@ -628,6 +634,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 \ @@ -756,6 +763,7 @@ EXTRA_CLEAN := $(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..19703e250549 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_no_cfi.c @@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */ +#include <test_progs.h> +#include <testing_helpers.h> + +static void load_bpf_test_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"); +} + +void test_struct_ops_no_cfi(void) +{ + if (test__start_subtest("load_bpf_test_no_cfi")) + load_bpf_test_no_cfi(); +} + 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] 7+ messages in thread
end of thread, other threads:[~2024-02-22 1:12 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-21 7:52 [PATCH bpf-next v4 0/3] Check cfi_stubs before registering a struct_ops type thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 1/3] bpf, net: allow passing NULL prog to check_member thinker.li 2024-02-21 7:52 ` [PATCH bpf-next v4 2/3] bpf: Check cfi_stubs before registering a struct_ops type thinker.li 2024-02-21 18:25 ` Martin KaFai Lau 2024-02-21 23:13 ` Kui-Feng Lee 2024-02-22 1:11 ` Kui-Feng Lee 2024-02-21 7:52 ` [PATCH bpf-next v4 3/3] selftests/bpf: Test case for lacking CFI stub functions thinker.li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox