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,
--
2.45.2> 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
On Wed, Dec 31, 2025 at 4:28 PM Caleb Sander Mateos
<csander@purestorage.com> wrote:
>
> 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.
You're saying that void const * cfi_stubs; pointing to non-const
__bpf_testmod_ops is somehow ok? No. This right into undefined behavior.
Not going to allow that.
On Wed, Dec 31, 2025 at 6:10 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Dec 31, 2025 at 4:28 PM Caleb Sander Mateos
> <csander@purestorage.com> wrote:
> >
> > 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.
>
> You're saying that void const * cfi_stubs; pointing to non-const
> __bpf_testmod_ops is somehow ok? No. This right into undefined behavior.
> Not going to allow that.
How is that undefined behavior? Wouldn't the following be UB by the
same reasoning?
void takes_const(const int *x);
void f(void)
{
int not_const = 123;
takes_const(¬_const);
}
A const-qualified pointer type just prevents that pointer from being
used to mutate the memory it points to. It doesn't guarantee that the
memory it points to is marked readonly.
© 2016 - 2026 Red Hat, Inc.