Thread (30 messages) 30 messages, 6 authors, 2020-01-16

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

From: Ard Biesheuvel <hidden>
Date: 2020-01-15 13:36:57

On Wed, 15 Jan 2020 at 13:42, Will Deacon [off-list ref] wrote:
On Wed, Jan 15, 2020 at 12:07:03PM +0000, Mark Brown wrote:
quoted
On Wed, Jan 15, 2020 at 09:16:16AM +0000, Will Deacon wrote:
quoted
On Wed, Jan 15, 2020 at 08:48:46AM +0100, Ard Biesheuvel wrote:
quoted
quoted
quoted
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
quoted
quoted
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...
quoted
quoted
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).
quoted
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.
Patches #1 and #3 are fine with me, modulo the HWCAP bit which I don't
deeply care about.

But the way this patch works around our workaround for mismatched RNG
caps between cores doesn't make sense to me.
arch_get_random_seed_long() should just have some out of line __init
path that gets invoked only during early boot, exactly how we are
using it in patch #3 to seed KASLR, where we don't care about whether
or not other CPUs have the extension. (Note that rand_initialize() is
called very early, way before the point where we have to care about
being scheduled from a CPU with RNG to one without)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help