On Wed, Dec 31, 2025 at 6:10 PM Alexei Starovoitov
<[email protected]> wrote:
>
> On Wed, Dec 31, 2025 at 4:28 PM Caleb Sander Mateos
> <[email protected]> wrote:
> >
> > On Wed, Dec 31, 2025 at 10:13 AM Alexei Starovoitov
> > <[email protected]> wrote:
> > >
> > > On Wed, Dec 31, 2025 at 10:09 AM Caleb Sander Mateos
> > > <[email protected]> wrote:
> > > >
> > > > On Wed, Dec 31, 2025 at 10:04 AM <[email protected]> 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.
