Re: [PATCH] Fix xoring of arch_get_random_long into crng->state array

From: Neil Horman
Date: Wed May 29 2019 - 11:55:58 EST


On Wed, May 29, 2019 at 01:51:24PM +0000, David Laight wrote:
> From: Neil Horman
> > Sent: 29 May 2019 14:42
> > On Tue, Apr 02, 2019 at 06:00:25PM -0400, Neil Horman wrote:
> > > When _crng_extract is called, any arch that has a registered
> > > arch_get_random_long method, attempts to mix an unsigned long value into
> > > the crng->state buffer, it only mixes in 32 of the 64 bits available,
> > > because the state buffer is an array of u32 values, even though 2 u32
> > > are expected to be filled (owing to the fact that it expects indexes 14
> > > and 15 to be filled).
> > >
> > > Bring the expected behavior into alignment by casting index 14 to an
> > > unsignled long pointer, and xoring that in instead.
> ...
> > > diff --git a/drivers/char/random.c b/drivers/char/random.c
> > > index 38c6d1af6d1c..8178618458ac 100644
> > > --- a/drivers/char/random.c
> > > +++ b/drivers/char/random.c
> > > @@ -975,14 +975,16 @@ static void _extract_crng(struct crng_state *crng,
> > > __u8 out[CHACHA_BLOCK_SIZE])
> > > {
> > > unsigned long v, flags;
> > > -
> > > + unsigned long *archrnd;
> > > if (crng_ready() &&
> > > (time_after(crng_global_init_time, crng->init_time) ||
> > > time_after(jiffies, crng->init_time + CRNG_RESEED_INTERVAL)))
> > > crng_reseed(crng, crng == &primary_crng ? &input_pool : NULL);
> > > spin_lock_irqsave(&crng->lock, flags);
> > > - if (arch_get_random_long(&v))
> > > - crng->state[14] ^= v;
> > > + if (arch_get_random_long(&v)) {
> > > + archrnd = (unsigned long *)&crng->state[14];
> > > + *archrnd ^= v;
> > > + }
>
> Isn't that likely to generate a misaligned memory access?
>
I'm not quite sure how it would, crng->state is an array of _u32's, and so every
even element should be on a 64 bit boundary.

Neil

> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>