struct bpf_struct_ops's cfi_stubs field is used as a readonly pointer but has type void *. Change its type to void const * to allow it to point to readonly global memory. Update the struct_ops implementations to declare their cfi_stubs global variables as const.
Caleb Sander Mateos (5): bpf: use const pointer for struct_ops cfi_stubs HID: bpf: make __bpf_hid_bpf_ops const sched_ext: make __bpf_ops_sched_ext_ops const net: make cfi_stubs globals const selftests/bpf: make cfi_stubs globals const
drivers/hid/bpf/hid_bpf_struct_ops.c | 2 +- include/linux/bpf.h | 2 +- kernel/bpf/bpf_struct_ops.c | 6 +++--- kernel/sched/ext.c | 2 +- net/bpf/bpf_dummy_struct_ops.c | 2 +- net/ipv4/bpf_tcp_ca.c | 2 +- net/sched/bpf_qdisc.c | 2 +- net/smc/smc_hs_bpf.c | 2 +- .../testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c | 2 +- tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 10 +++++----- 10 files changed, 16 insertions(+), 16 deletions(-)
struct bpf_struct_ops's cfi_stubs field is used as a readonly pointer but has type void *. Change its type to void const * to allow it to point to readonly global memory. Change the void ** casts of cfi_stubs to void * const * accordingly.
Signed-off-by: Caleb Sander Mateos csander@purestorage.com --- include/linux/bpf.h | 2 +- kernel/bpf/bpf_struct_ops.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 4e7d72dfbcd4..d74189ea1066 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1977,11 +1977,11 @@ struct bpf_struct_ops { void *kdata, const void *udata); int (*reg)(void *kdata, struct bpf_link *link); void (*unreg)(void *kdata, struct bpf_link *link); int (*update)(void *kdata, void *old_kdata, struct bpf_link *link); int (*validate)(void *kdata); - void *cfi_stubs; + void const *cfi_stubs; struct module *owner; const char *name; struct btf_func_model func_models[BPF_STRUCT_OPS_MAX_NR_MEMBERS]; };
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c index c43346cb3d76..42cfc3e0bc68 100644 --- a/kernel/bpf/bpf_struct_ops.c +++ b/kernel/bpf/bpf_struct_ops.c @@ -321,11 +321,11 @@ static bool is_module_member(const struct btf *btf, u32 id) return !strcmp(btf_name_by_offset(btf, t->name_off), "module"); }
int bpf_struct_ops_supported(const struct bpf_struct_ops *st_ops, u32 moff) { - void *func_ptr = *(void **)(st_ops->cfi_stubs + moff); + void *func_ptr = *(void * const *)(st_ops->cfi_stubs + moff);
return func_ptr ? 0 : -ENOTSUPP; }
int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc, @@ -444,11 +444,11 @@ int bpf_struct_ops_desc_init(struct bpf_struct_ops_desc *st_ops_desc, mname, st_ops->name); err = -EINVAL; goto errout; }
- stub_func_addr = *(void **)(st_ops->cfi_stubs + moff); + stub_func_addr = *(void * const *)(st_ops->cfi_stubs + moff); err = prepare_arg_info(btf, st_ops->name, mname, func_proto, stub_func_addr, arg_info + i); if (err) goto errout; @@ -833,11 +833,11 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key, *pksym++ = ksym;
trampoline_start = image_off; err = bpf_struct_ops_prepare_trampoline(tlinks, link, &st_ops->func_models[i], - *(void **)(st_ops->cfi_stubs + moff), + *(void * const *)(st_ops->cfi_stubs + moff), &image, &image_off, st_map->image_pages_cnt < MAX_TRAMP_IMAGE_PAGES); if (err) goto reset_unlock;
Now that struct bpf_struct_ops's cfi_stubs field is a const pointer, declare the __bpf_hid_bpf_ops global variable it points to as const. This allows the global variable to be placed in readonly memory.
Signed-off-by: Caleb Sander Mateos csander@purestorage.com --- drivers/hid/bpf/hid_bpf_struct_ops.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c index 702c22fae136..30ddcf78e0ea 100644 --- a/drivers/hid/bpf/hid_bpf_struct_ops.c +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c @@ -286,11 +286,11 @@ static int __hid_bpf_hw_request(struct hid_bpf_ctx *ctx, unsigned char reportnum static int __hid_bpf_hw_output_report(struct hid_bpf_ctx *ctx, u64 source) { return 0; }
-static struct hid_bpf_ops __bpf_hid_bpf_ops = { +static const struct hid_bpf_ops __bpf_hid_bpf_ops = { .hid_device_event = __hid_bpf_device_event, .hid_rdesc_fixup = __hid_bpf_rdesc_fixup, .hid_hw_request = __hid_bpf_hw_request, .hid_hw_output_report = __hid_bpf_hw_output_report, };
Now that struct bpf_struct_ops's cfi_stubs field is a const pointer, declare the __bpf_ops_sched_ext_ops global variable it points to as const. This allows the global variable to be placed in readonly memory.
Signed-off-by: Caleb Sander Mateos csander@purestorage.com --- kernel/sched/ext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c index 94164f2dec6d..af8250b64f47 100644 --- a/kernel/sched/ext.c +++ b/kernel/sched/ext.c @@ -5336,11 +5336,11 @@ static s32 sched_ext_ops__init(void) { return -EINVAL; } static void sched_ext_ops__exit(struct scx_exit_info *info) {} static void sched_ext_ops__dump(struct scx_dump_ctx *ctx) {} static void sched_ext_ops__dump_cpu(struct scx_dump_ctx *ctx, s32 cpu, bool idle) {} static void sched_ext_ops__dump_task(struct scx_dump_ctx *ctx, struct task_struct *p) {}
-static struct sched_ext_ops __bpf_ops_sched_ext_ops = { +static const struct sched_ext_ops __bpf_ops_sched_ext_ops = { .select_cpu = sched_ext_ops__select_cpu, .enqueue = sched_ext_ops__enqueue, .dequeue = sched_ext_ops__dequeue, .dispatch = sched_ext_ops__dispatch, .tick = sched_ext_ops__tick,
Now that struct bpf_struct_ops's cfi_stubs field is a const pointer, declare the __bpf_bpf_dummy_ops, __bpf_ops_tcp_congestion_ops, __bpf_ops_qdisc_ops, and __smc_bpf_hs_ctrl global variables it points to as const. This allows the global variables to be placed in readonly memory.
Signed-off-by: Caleb Sander Mateos csander@purestorage.com --- net/bpf/bpf_dummy_struct_ops.c | 2 +- net/ipv4/bpf_tcp_ca.c | 2 +- net/sched/bpf_qdisc.c | 2 +- net/smc/smc_hs_bpf.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c index 812457819b5a..198152dbce9a 100644 --- a/net/bpf/bpf_dummy_struct_ops.c +++ b/net/bpf/bpf_dummy_struct_ops.c @@ -296,11 +296,11 @@ static int bpf_dummy_test_2(struct bpf_dummy_ops_state *cb, int a1, unsigned sho static int bpf_dummy_test_sleepable(struct bpf_dummy_ops_state *cb) { return 0; }
-static struct bpf_dummy_ops __bpf_bpf_dummy_ops = { +static const struct bpf_dummy_ops __bpf_bpf_dummy_ops = { .test_1 = bpf_dummy_ops__test_1, .test_2 = bpf_dummy_test_2, .test_sleepable = bpf_dummy_test_sleepable, };
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c index e01492234b0b..bd2ce4ff1762 100644 --- a/net/ipv4/bpf_tcp_ca.c +++ b/net/ipv4/bpf_tcp_ca.c @@ -306,11 +306,11 @@ static void __bpf_tcp_ca_init(struct sock *sk)
static void __bpf_tcp_ca_release(struct sock *sk) { }
-static struct tcp_congestion_ops __bpf_ops_tcp_congestion_ops = { +static const struct tcp_congestion_ops __bpf_ops_tcp_congestion_ops = { .ssthresh = bpf_tcp_ca_ssthresh, .cong_avoid = bpf_tcp_ca_cong_avoid, .set_state = bpf_tcp_ca_set_state, .cwnd_event = bpf_tcp_ca_cwnd_event, .in_ack_event = bpf_tcp_ca_in_ack_event, diff --git a/net/sched/bpf_qdisc.c b/net/sched/bpf_qdisc.c index adcb618a2bfc..8f9a6440f113 100644 --- a/net/sched/bpf_qdisc.c +++ b/net/sched/bpf_qdisc.c @@ -427,11 +427,11 @@ static void Qdisc_ops__reset(struct Qdisc *sch)
static void Qdisc_ops__destroy(struct Qdisc *sch) { }
-static struct Qdisc_ops __bpf_ops_qdisc_ops = { +static const struct Qdisc_ops __bpf_ops_qdisc_ops = { .enqueue = Qdisc_ops__enqueue, .dequeue = Qdisc_ops__dequeue, .init = Qdisc_ops__init, .reset = Qdisc_ops__reset, .destroy = Qdisc_ops__destroy, diff --git a/net/smc/smc_hs_bpf.c b/net/smc/smc_hs_bpf.c index 063d23d85850..5c562e2a15be 100644 --- a/net/smc/smc_hs_bpf.c +++ b/net/smc/smc_hs_bpf.c @@ -60,11 +60,11 @@ static int __smc_bpf_stub_set_tcp_option_cond(const struct tcp_sock *tp, struct inet_request_sock *ireq) { return 1; }
-static struct smc_hs_ctrl __smc_bpf_hs_ctrl = { +static const struct smc_hs_ctrl __smc_bpf_hs_ctrl = { .syn_option = __smc_bpf_stub_set_tcp_option, .synack_option = __smc_bpf_stub_set_tcp_option_cond, };
static int smc_bpf_hs_ctrl_init(struct btf *btf) { return 0; }
Now that struct bpf_struct_ops's cfi_stubs field is a const pointer, declare the __test_no_cif_ops, __bpf_testmod_ops*, st_ops_cfi_stubs, and multi_st_ops_cfi_stubs global variables it points to as const. This tests that BPF struct_ops implementations are allowed to declare cfi_stubs global variables as const.
Signed-off-by: Caleb Sander Mateos csander@purestorage.com --- .../testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c | 2 +- tools/testing/selftests/bpf/test_kmods/bpf_testmod.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c b/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c index 948eb3962732..1d76912f1a45 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_test_no_cfi.c @@ -39,11 +39,11 @@ 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 __test_no_cif_ops = { +static const struct bpf_test_no_cfi_ops __test_no_cif_ops = { .fn_1 = bpf_test_no_cfi_ops__fn_1, .fn_2 = bpf_test_no_cfi_ops__fn_2, };
static struct bpf_struct_ops test_no_cif_ops = { diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 90c4b1a51de6..5e460b1dbdb6 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c @@ -295,11 +295,11 @@ static int bpf_testmod_test_3(void) static int bpf_testmod_test_4(void) { return 0; }
-static struct bpf_testmod_ops3 __bpf_testmod_ops3 = { +static const struct bpf_testmod_ops3 __bpf_testmod_ops3 = { .test_1 = bpf_testmod_test_3, .test_2 = bpf_testmod_test_4, };
static void bpf_testmod_test_struct_ops3(void) @@ -1273,11 +1273,11 @@ bpf_testmod_ops__test_return_ref_kptr(int dummy, struct task_struct *task__ref, struct cgroup *cgrp) { return NULL; }
-static struct bpf_testmod_ops __bpf_testmod_ops = { +static const struct bpf_testmod_ops __bpf_testmod_ops = { .test_1 = bpf_testmod_test_1, .test_2 = bpf_testmod_test_2, .test_maybe_null = bpf_testmod_ops__test_maybe_null, .test_refcounted = bpf_testmod_ops__test_refcounted, .test_return_ref_kptr = bpf_testmod_ops__test_return_ref_kptr, @@ -1300,11 +1300,11 @@ static int bpf_dummy_reg2(void *kdata, struct bpf_link *link)
ops->test_1(); return 0; }
-static struct bpf_testmod_ops2 __bpf_testmod_ops2 = { +static const struct bpf_testmod_ops2 __bpf_testmod_ops2 = { .test_1 = bpf_testmod_test_1, };
struct bpf_struct_ops bpf_testmod_ops2 = { .verifier_ops = &bpf_testmod_verifier_ops, @@ -1547,11 +1547,11 @@ static const struct bpf_verifier_ops st_ops_verifier_ops = { .gen_prologue = st_ops_gen_prologue, .gen_epilogue = st_ops_gen_epilogue, .get_func_proto = bpf_base_func_proto, };
-static struct bpf_testmod_st_ops st_ops_cfi_stubs = { +static const struct bpf_testmod_st_ops st_ops_cfi_stubs = { .test_prologue = bpf_test_mod_st_ops__test_prologue, .test_epilogue = bpf_test_mod_st_ops__test_epilogue, .test_pro_epilogue = bpf_test_mod_st_ops__test_pro_epilogue, };
@@ -1715,11 +1715,11 @@ static void multi_st_ops_unreg(void *kdata, struct bpf_link *link) static int bpf_testmod_multi_st_ops__test_1(struct st_ops_args *args) { return 0; }
-static struct bpf_testmod_multi_st_ops multi_st_ops_cfi_stubs = { +static const struct bpf_testmod_multi_st_ops multi_st_ops_cfi_stubs = { .test_1 = bpf_testmod_multi_st_ops__test_1, };
struct bpf_struct_ops testmod_multi_st_ops = { .verifier_ops = &bpf_testmod_verifier_ops,
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 90c4b1a51de6..5e460b1dbdb6 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
@@ -1275,7 +1275,7 @@ bpf_testmod_ops__test_return_ref_kptr(int dummy, struct task_struct *task__ref, return NULL; }
-static struct bpf_testmod_ops __bpf_testmod_ops = { +static const struct bpf_testmod_ops __bpf_testmod_ops = { .test_1 = bpf_testmod_test_1, .test_2 = bpf_testmod_test_2,
Is it safe to make __bpf_testmod_ops const here? In bpf_testmod_init(), this struct is modified at runtime:
tramp = (void **)&__bpf_testmod_ops.tramp_1; while (tramp <= (void **)&__bpf_testmod_ops.tramp_40) *tramp++ = bpf_testmod_tramp;
Writing to a const-qualified object is undefined behavior and may cause a protection fault when the compiler places this in read-only memory. Would the module fail to load on systems where .rodata is actually read-only?
--- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20624206229
On Wed, Dec 31, 2025 at 10:04 AM bot+bpf-ci@kernel.org wrote:
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 90c4b1a51de6..5e460b1dbdb6 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
@@ -1275,7 +1275,7 @@ bpf_testmod_ops__test_return_ref_kptr(int dummy, struct task_struct *task__ref, return NULL; }
-static struct bpf_testmod_ops __bpf_testmod_ops = { +static const struct bpf_testmod_ops __bpf_testmod_ops = { .test_1 = bpf_testmod_test_1, .test_2 = bpf_testmod_test_2,
Is it safe to make __bpf_testmod_ops const here? In bpf_testmod_init(), this struct is modified at runtime:
tramp = (void **)&__bpf_testmod_ops.tramp_1; while (tramp <= (void **)&__bpf_testmod_ops.tramp_40) *tramp++ = bpf_testmod_tramp;Writing to a const-qualified object is undefined behavior and may cause a protection fault when the compiler places this in read-only memory. Would the module fail to load on systems where .rodata is actually read-only?
Yup, that's indeed the bug caught by KASAN. Missed this mutation at init time, I'll leave __bpf_testmod_ops as mutable.
Thanks, Caleb
AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20624206229
On Wed, Dec 31, 2025 at 10:09 AM Caleb Sander Mateos csander@purestorage.com wrote:
On Wed, Dec 31, 2025 at 10:04 AM bot+bpf-ci@kernel.org wrote:
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 90c4b1a51de6..5e460b1dbdb6 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
@@ -1275,7 +1275,7 @@ bpf_testmod_ops__test_return_ref_kptr(int dummy, struct task_struct *task__ref, return NULL; }
-static struct bpf_testmod_ops __bpf_testmod_ops = { +static const struct bpf_testmod_ops __bpf_testmod_ops = { .test_1 = bpf_testmod_test_1, .test_2 = bpf_testmod_test_2,
Is it safe to make __bpf_testmod_ops const here? In bpf_testmod_init(), this struct is modified at runtime:
tramp = (void **)&__bpf_testmod_ops.tramp_1; while (tramp <= (void **)&__bpf_testmod_ops.tramp_40) *tramp++ = bpf_testmod_tramp;Writing to a const-qualified object is undefined behavior and may cause a protection fault when the compiler places this in read-only memory. Would the module fail to load on systems where .rodata is actually read-only?
Yup, that's indeed the bug caught by KASAN. Missed this mutation at init time, I'll leave __bpf_testmod_ops as mutable.
No. You're missing the point. The whole patch set is no go. The pointer to cfi stub can be updated just as well.
On Wed, Dec 31, 2025 at 10:13 AM Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
On Wed, Dec 31, 2025 at 10:09 AM Caleb Sander Mateos csander@purestorage.com wrote:
On Wed, Dec 31, 2025 at 10:04 AM bot+bpf-ci@kernel.org wrote:
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 90c4b1a51de6..5e460b1dbdb6 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
@@ -1275,7 +1275,7 @@ bpf_testmod_ops__test_return_ref_kptr(int dummy, struct task_struct *task__ref, return NULL; }
-static struct bpf_testmod_ops __bpf_testmod_ops = { +static const struct bpf_testmod_ops __bpf_testmod_ops = { .test_1 = bpf_testmod_test_1, .test_2 = bpf_testmod_test_2,
Is it safe to make __bpf_testmod_ops const here? In bpf_testmod_init(), this struct is modified at runtime:
tramp = (void **)&__bpf_testmod_ops.tramp_1; while (tramp <= (void **)&__bpf_testmod_ops.tramp_40) *tramp++ = bpf_testmod_tramp;Writing to a const-qualified object is undefined behavior and may cause a protection fault when the compiler places this in read-only memory. Would the module fail to load on systems where .rodata is actually read-only?
Yup, that's indeed the bug caught by KASAN. Missed this mutation at init time, I'll leave __bpf_testmod_ops as mutable.
No. You're missing the point. The whole patch set is no go. The pointer to cfi stub can be updated just as well.
Do you mean the BPF core code would modify the struct pointed to by cfi_stubs? Or some BPF struct_ops implementation (like this one in bpf_testmod.c) would modify it? If you're talking about the BPF core code, could you point out where this happens? I couldn't find it when looking through the handful of uses of cfi_stubs (see patch 1/5). Or are you talking about some hypothetical future code that would write through the cfi_stubs pointer? If you're talking about a struct_ops implementation, I certainly agree it could modify the struct pointed to by cfi_stubs (before calling register_bpf_struct_ops()). But then the struct_ops implementation doesn't have to declare the global variable as const. A non-const pointer is allowed anywhere a const pointer is expected.
Thanks, Caleb
linux-kselftest-mirror@lists.linaro.org
-
Alexei Starovoitov -
bot+bpf-ci＠kernel.org -
Caleb Sander Mateos