Re: [PATCH v2 2/9] random: get rid of secondary crngs

Jason A. Donenfeld
2022-02-09T10:40:25Z

Hey Dominik,

On Wed, Feb 9, 2022 at 9:31 AM Dominik Brodowski
<linux@dominikbrodowski.net> wrote:
>
> Just one question (which was already present in the past, but...):
>
> > - WRITE_ONCE(crng->init_time, jiffies);
> > - spin_unlock_irqrestore(&crng->lock, flags);
> > - if (crng == &primary_crng && crng_init < 2)
> > - crng_finalize_init();
> > + WRITE_ONCE(primary_crng.init_time, jiffies);
> > + spin_unlock_irqrestore(&primary_crng.lock, flags);
> > + if (crng_init < 2) {
> > + invalidate_batched_entropy();
> > + crng_init = 2;
>
> Might this branch be taken twice if crng_reseed() is called concurrently
> twice? If so, we'd need to increment crng_init while holding the lock,
> such as I suggested in my patch "random: fix locking for crng_init in
> crng_reseed()". But that can be deferred to an additional patch.

Yes absolutely it can and absolutely we should. As a matter of fact,
your patch that you reference was kind of the straw that broke the
camel's back that motivated me to do this patchset now rather than
deferring it. Now we can fix the race you speak of without having to
split the functions and complicate the call graph even more. Do you
think you could rebase your patch on this branch and re-send? I'll
happily apply.

Jason