linux-kernel - Re: [PATCH] random: defer use of bootloader randomness to random

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: <Yp8+6Y+bEcmR1LS0@zx2c4.com>
Date:   Tue, 7 Jun 2022 14:04:57 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Stephen Boyd <swboyd@...omium.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Russell King <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>,
        Phil Elwell <phil@...pberrypi.com>
Subject: Re: [PATCH] random: defer use of bootloader randomness to
 random_init()

Hi Ard,

On Tue, Jun 07, 2022 at 01:55:37PM +0200, Ard Biesheuvel wrote:
> This is not going to work, I'm afraid - please see below.
>
> The next line says it all, really: the seed is in a firmware table
> somewhere, and only gets mapped temporarily here. Note that we cannot
> copy it either, as we are running way before we have discovered where
> RAM is to begin with.

Oh, darn. Okay. The v2 might be a bit more palpable then. It mixes
immediately, but defers crediting:

[1] https://lore.kernel.org/lkml/20220607113238.769088-1-Jason@zx2c4.com/

> - Even if very convincing replies can be given to the previous two
> points, wouldn't it be betterr to simply revert the -stable backport
> that introduces the use of the static key, and find a robust and
> portable solution for after v5.19?

This has already been done:

[2] https://lore.kernel.org/stable/20220607084005.666059-1-Jason@zx2c4.com/

And mentioned here:

[3] https://lore.kernel.org/lkml/CAHmME9rJif3ydZuFJcSjPxkGMofZkbu2PXcHBF23OWVgGQ4c+A@mail.gmail.com/

You're on this thread.

> So could we please go back to some basic questions here;
> - Why do we need/want a static key here to begin with? Is is for performance?
> - Why do we need to enable this static key so early?

We don't need to enable it especially early. I've now sent three
different approaches for deferring it until later and you suggested one.
The first of mine is kind of ugly (checking static_key_initialized and
such at different points).  Your suggested one after that did the same
but deferred into crng_reseed(), which I'm not a fan of. My second one
is this patch, which is flawed for the reason you pointed out. But
perhaps my third one is the right amount of simple and okay? That's the
one I linked up top, [1]. Let me know what you think of that.

My motivation for not wanting to defer it is that if the arch solution
winds up being easy and straight forward (as it was for arm64), then it
would be nice to not need to clutter up random.c as a result. But if
the arch solution winds up looking fragile or problematic somehow, then
one of these solutions should do the trick. In particular [1] seems nice
enough that it doesn't really even clutter things up too much as feared.

Jason