On Wed, May 29, 2019 at 03:57:07PM +0000, David Laight wrote: > From: Neil Horman [mailto:nhor...@tuxdriver.com] > > Sent: 29 May 2019 16:52 > > 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. > > Only if the first item is aligned.... > Add a u32 before it and you'll probably flip the alignment. > Sure (assuming no padding by the compiler of leading elements), but thats not the case here, state is the first element in the array. I suppose we could add an __attribute__((aligned,8)) to the element if you think it would help
Neil > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 > 1PT, UK > Registration No: 1397386 (Wales) > >
