linux-kernel - Re: [PATCH 4.16 163/196] random: fix crng

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: <20180427163443.fdsixdsrwwkycuhh@xps>
Date:   Fri, 27 Apr 2018 11:34:43 -0500
From:   Dan Rue <dan.rue@...aro.org>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Jann Horn <jannh@...gle.com>, Theodore Tso <tytso@....edu>,
        stable@...nel.org, Thierry Escande <thierry.escande@...aro.org>,
        Nicolas Dechesne <nicolas.dechesne@...aro.org>
Subject: Re: [PATCH 4.16 163/196] random: fix crng_ready() test

On Sun, Apr 22, 2018 at 03:53:03PM +0200, Greg Kroah-Hartman wrote:
> 4.16-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Theodore Ts'o <tytso@....edu>
> 
> commit 43838a23a05fbd13e47d750d3dfd77001536dd33 upstream.
> 
> The crng_init variable has three states:
> 
> 0: The CRNG is not initialized at all
> 1: The CRNG has a small amount of entropy, hopefully good enough for
>    early-boot, non-cryptographical use cases
> 2: The CRNG is fully initialized and we are sure it is safe for
>    cryptographic use cases.
> 
> The crng_ready() function should only return true once we are in the
> last state.  This addresses CVE-2018-1108.
> 
> Reported-by: Jann Horn <jannh@...gle.com>
> Fixes: e192be9d9a30 ("random: replace non-blocking pool...")
> Cc: stable@...nel.org # 4.8+
> Signed-off-by: Theodore Ts'o <tytso@....edu>
> Reviewed-by: Jann Horn <jannh@...gle.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

This patch has caused a regression on 4.16 using kselftest lib/printf.sh
- specifically, when it runs "/sbin/modprobe test_printf". This
regression has been detected on arm64 dragonboard 410c (not seen on
other arm64 or x86_64 devices).

    /sbin/modprobe test_printf
    [   22.725551] test_printf: hashing plain 'p' has unexpected format
    [   22.726031] test_printf: failed 1 out of 236 tests
    modprobe: ERROR: could not insert 'test_printf': Invalid argument

This problem has not been seen on 4.9 or 4.14 under the same
conditions.

Dan

> 
> ---
>  drivers/char/random.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -427,7 +427,7 @@ struct crng_state primary_crng = {
>   * its value (from 0->1->2).
>   */
>  static int crng_init = 0;
> -#define crng_ready() (likely(crng_init > 0))
> +#define crng_ready() (likely(crng_init > 1))
>  static int crng_init_cnt = 0;
>  #define CRNG_INIT_CNT_THRESH (2*CHACHA20_KEY_SIZE)
>  static void _extract_crng(struct crng_state *crng,
> @@ -793,7 +793,7 @@ static int crng_fast_load(const char *cp
>  
>  	if (!spin_trylock_irqsave(&primary_crng.lock, flags))
>  		return 0;
> -	if (crng_ready()) {
> +	if (crng_init != 0) {
>  		spin_unlock_irqrestore(&primary_crng.lock, flags);
>  		return 0;
>  	}
> @@ -855,7 +855,7 @@ static void _extract_crng(struct crng_st
>  {
>  	unsigned long v, flags;
>  
> -	if (crng_init > 1 &&
> +	if (crng_ready() &&
>  	    time_after(jiffies, crng->init_time + CRNG_RESEED_INTERVAL))
>  		crng_reseed(crng, crng == &primary_crng ? &input_pool : NULL);
>  	spin_lock_irqsave(&crng->lock, flags);
> @@ -1141,7 +1141,7 @@ void add_interrupt_randomness(int irq, i
>  	fast_mix(fast_pool);
>  	add_interrupt_bench(cycles);
>  
> -	if (!crng_ready()) {
> +	if (unlikely(crng_init == 0)) {
>  		if ((fast_pool->count >= 64) &&
>  		    crng_fast_load((char *) fast_pool->pool,
>  				   sizeof(fast_pool->pool))) {
> @@ -2214,7 +2214,7 @@ void add_hwgenerator_randomness(const ch
>  {
>  	struct entropy_store *poolp = &input_pool;
>  
> -	if (!crng_ready()) {
> +	if (unlikely(crng_init == 0)) {
>  		crng_fast_load(buffer, count);
>  		return;
>  	}
> 
>