linux-kernel - Re: [PATCH v5] random: remove early archrandom abstraction

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: <Y2EaG7Zo2jiyKUWB@FVFF77S0Q05N>
Date:   Tue, 1 Nov 2022 13:07:39 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Will Deacon <will@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>
Subject: Re: [PATCH v5] random: remove early archrandom abstraction

On Tue, Nov 01, 2022 at 02:02:58PM +0100, Jason A. Donenfeld wrote:
> Hi Mark,
> 
> On Tue, Nov 01, 2022 at 12:36:07PM +0000, Mark Rutland wrote:
> > Hi Jason,
> > 
> > Sorry for joining this late...
> > 
> > On Tue, Nov 01, 2022 at 01:25:28PM +0100, Jason A. Donenfeld wrote:
> > > The arch_get_random*_early() abstraction is not completely useful and
> > > adds complexity, because it's not a given that there will be no calls to
> > > arch_get_random*() between random_init_early(), which uses
> > > arch_get_random*_early(), and init_cpu_features(). During that gap,
> > > crng_reseed() might be called, which uses arch_get_random*(), since it's
> > > mostly not init code.
> > 
> > The original rationale for arch_get_random*_early() was just to seed the RNG
> > more robustly rather than to feed every possible arch_get_random() call made
> > early in the boot flow, and the rationale for having a separate functions was
> > that it was trivial to see by inspection that it was (only) called in the
> > expected places.
> > 
> > I'm not wedded to arch_get_random*_early() specifically, but I do think that
> > having arch_get_random() behave differently depending on which phase of boot
> > we're in has more scope for error than having a separate call of some sort.
> > 
> > Other than removing the lines below, what chages is this going to permit?
> 
> Firstly, the issue with the API is having to remember to use it! There's
> already been a bug from forgetting to use the _early() call during some
> refactoring, and I doubt it'll be the last.
> 
> But also, functions such as crng_reseed()->extract_entropy() wind up
> being called in both early contexts and normal contexts. It's not
> feasible to have different paths there, so by having two functions,
> we miss out on having access during early boot.
> 
> So I don't want a separate call, both for the API complexity reasons,
> and because it doesn't really work as intended in the end.

If it's too painful to use separate paths, then fair enough. Thanks for the
summary!

Mark.