Re: [PATCH v5] random: remove early archrandom abstraction

Mark Rutland
2022-11-01T13:07:49Z

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.