On Mon, Nov 10, 2025 at 12:40:17PM +0100, Jason A. Donenfeld wrote:
> On Mon, Nov 10, 2025 at 12:24:13PM +0100, Thomas Weißschuh wrote:
> > > > > For example, one clean way of
> > > > > doing that would be to make vdso_k_rng_data always valid by having it
> > > > > initially point to __initdata memory, and then when it's time to
> > > > > initialize the real datapage, memcpy() the __initdata memory to the
> > > > > new
> > > > > specially allocated memory. Then we don't need the complex state
> > > > > tracking that this commit and the prior one introduce.
> > > >
> > > > Wouldn't that require synchronization between the update path and the
> > > > memcpy()
> > > > path? Also if the pointer is going to change at some point we'll
> > > > probably need
> > > > to use READ_ONCE()/WRITE_ONCE(). In general I would be happy about a
> > > > cleaner
> > > > solution for this but didn't find a great one.
> > >
> > > This is still before userspace has started, and interrupts are disabled,
> > > so I don't think so?
> >
> > Interrupts being disabled is a good point. But we are still leaking
> > implementation details from the random code into the vdso datastore.
>
> It wouldn't. You do this generically with memcpy().
With "implementation details" I meant the fact that it is fine to swap out the
datapage behind its back. And the fact that the memcpy() can not introduce any
races.
> > > Also, you only care about being after
> > > mm_core_init(), right? So move your thing before sched_init() and then
> > > you'll really have nothing to worry about.
> >
> > The callchains random_init_early() -> crng_reseed()/_credit_init_bits()
> > could
> > still touch the datapage before it is allocated.
> > Adding conditionals to prevent those is essentially what my patch does.
>
> I think I wasn't very clear in my proposal, because this isn't an issue
> in it.
I interpreted your previous mail as two different proposals:
1) do the memcpy() thing
2) move the page allocation after mm_core_init()
Now it makes more sense.
> Global scope:
>
> static struct vdso_rng_data placeholder_vdso_k_rng_data __initdata;
> struct vdso_rng_data *vdso_k_rng_data = &placeholder_vdso_k_rng_data;
>
> Then,
>
> void __init vdso_setup_data_pages(void)
> {
> ...
> vdso_k_rng_data = blabla();
> ...
> memcpy(vdso_k_rng_data, &placeholder_vdso_k_rng_data,
> sizeof(*vdso_k_rng_data);
> ...
> }
>
> If vdso_setup_data_pages() is called early enough in init, this is safe
> to do, and then you don't need to muck up the random code with awful
> state machine ordering stuff.
Yes it is safe, but this safety is not obvious in my opinion.
However I'll use your proposal for the next revision.
Thomas
