linux-kernel - RE: [PATCH] Fix xoring of arch_get_random

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f13de0f3159a478796a8fe6c34dc00ce@AcuMS.aculab.com>
Date:   Wed, 29 May 2019 13:51:24 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Neil Horman' <nhorman@...driver.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     Steve Grubb <sgrubb@...hat.com>, Theodore Ts'o <tytso@....edu>,
        "Arnd Bergmann" <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: RE: [PATCH] Fix xoring of arch_get_random_long into crng->state array

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?

	David

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