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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHmME9rjNQuBzoenhQ8rh_zep5bW9jRKj2zncTUFSu5dy1smFQ@mail.gmail.com>
Date:   Wed, 9 Feb 2022 11:26:41 +0100
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Dominik Brodowski <linux@...inikbrodowski.net>
Cc:     Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Theodore Ts'o" <tytso@....edu>, Eric Biggers <ebiggers@...nel.org>
Subject: Re: [PATCH v2 2/9] random: get rid of secondary crngs

Hey Dominik,

On Wed, Feb 9, 2022 at 9:31 AM Dominik Brodowski
<linux@...inikbrodowski.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