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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2213 bytes --]

On Thu, Jan 09, 2020 at 08:33:25AM +1100, Richard Henderson wrote:
> On 1/9/20 6:41 AM, Mark Brown wrote:

> > +	for (i = 0; i < 16; i++)
> > +		if (__arm64_rndr(&val))
> > +			add_device_randomness(&val, sizeof(val));
> > +}

> This is not nearly the same thing as what crng_initialize does.  In particular,
> it's not going to advance crng_init at all.

That's right, but I think that's good enough to get us going here.  It
will add data into the pool so we're mitigating against a lack of per
device entropy which seems clearly better than doing nothing at all and
has no issues with integration with the decision about trusting the RNG
to provide entropy so it's safe.  The commit message does say we add
data rather than entropy, though I agree that on reflection the callback
isn't clearly named there and people not familiar with the random
subsystem will likely not notice the difference.

We could definitely improve the commit message a bit here or even drop
the patch but I think we're better off with this than without it, and
exposing the feature to userspace, allowing in kernel usage after init
and using it for KASAN are clear wins regardless of what we do with the
pool.  If we can do something that credits the entropy at boot that'd be
even better of course but I don't think that needs to block everything
else.

> You could use add_hwgenerator_randomness, but you have no way to honor the
> random.trust_cpu command-line parameter that way.

Right, that'd definitely be the wrong thing to do here.

> The only thing I can imagine that would satisfy MarkR's constraints is to have

The main issue he had was as far as I can tell with adding complexity to
the main runtime path which we now avoid, we now don't have anything
that needs to disable preemption or anything like that.

> a new archrandom.h interface, arch_get_random_boot_long().

An equivalent of device_add_randomness() that credits entropy would also
do the trick for adding entropy, it'd definitely be more hassle to
implement and quite possibly more trouble than it's worth compared to a
simple call like you suggest but it does have the advantage that you
know the core can't try to call it once we've got multiple CPUs up.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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