Re: [PATCH v10 2/3] arm64: random: Add data to pool from setup_arch()

[Will Deacon <will@kernel.org>]

On Wed, Jan 15, 2020 at 12:07:03PM +0000, Mark Brown wrote:
> On Wed, Jan 15, 2020 at 09:16:16AM +0000, Will Deacon wrote:
> > On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
> 
> > > > Note that we are only adding data here, it will be mixed into the pool
> > > > but won't be credited as entropy. There are currently no suitable
> > > > interfaces for that at present - extending the random code to provide
> 
> > > This is slightly unfortunate, as this way, we lose the ability to use
> > > random.trust_cpu=1 to get the entropy credited and initialize CRNG
> > > early.
> 
> Right.  OTOH that's a bit of a mess to do, I do have some
> thoughts but it's a bit of a mess trying to do it tastefully,
> especially when considering that you probably don't want an
> interface that it's easy for something to misuse.  The effort
> involved certainly seems large enough to handle separately.

Maybe, but see below...

> > Agreed. Do you think we should wait for that support before merging the
> > series? Given that I don't know of any CPUs implementing this extension,
> > we can probably afford not to rush this in.
> 
> It's implemented in at least the fast models already, not checked
> any of the other emulators, so there's some possibility of people
> using it while developing other things and hopefully at least
> some of the various CI systems will be including emulated
> platforms with newer extensions in their coverage so might gain
> some benefit from it.  Frankly the only reason I'm looking at
> this at all is that I'd written patch 3 because I was getting fed
> up with KASLR initialization being easily disabled when I was
> trying to test E0PD on the models (especially before I added the
> status print at boot to KASLR so this happened silently), having
> this in mainline would've helped considerably when working on
> that.

I was thinking specifically about users on silicon rather than developers
on simulators. (I could stick this on a branch for developers if necessary).

> I don't see any downside to having the code in mainline as is,
> even though it's not ideal it does make things better since if
> for some reason anyone does end up running this code on a system
> that has the feature they'll get at least some benefit from it
> even if nothing else happens.  The bulk of the code isn't going
> to change when the early init stuff gets improved and includes
> tables like cpufeature.h that make it annoying to hold out of
> tree, the bits that are going to change can just as well be
> worked on incrementally as held out of tree entirely and having
> the rest in means there's less friction doing that.

The usual downside that comes from merging patches with promises of fixing
them up later is that the motivating task gets marked as "done" somewhere,
the developer gets given something else to do and the updates never
materialise. That's not a dig at you; it's just the way these things tend
to work (I've certainly been on both sides of that coin).

If there was an urgency to this, I'd suggest merging a form of Richard's
code, as it appears to solve the technical issue of credited entropy whilst
leaving some room for subsequent cleanup. However, I think that makes it
even less likely that anybody will come back to do the cleanup because the
code will be perfectly functional, so I'd prefer to wait for a complete
solution unless you think it's not achievable for 5.7.

I'd also really like Ard's ack on anything relating to RNGs.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel