linux-kernel - Re: [PATCH v13 6/7] random: introduce generic vDSO getrandom() implementation

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: <Y6OHa196S8e1mImg@sol.localdomain>
Date:   Wed, 21 Dec 2022 14:23:39 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
        tglx@...utronix.de, linux-crypto@...r.kernel.org,
        linux-api@...r.kernel.org, x86@...nel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>,
        Carlos O'Donell <carlos@...hat.com>,
        Florian Weimer <fweimer@...hat.com>,
        Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>,
        Christian Brauner <brauner@...nel.org>
Subject: Re: [PATCH v13 6/7] random: introduce generic vDSO getrandom()
 implementation

On Wed, Dec 21, 2022 at 03:23:26PM +0100, Jason A. Donenfeld wrote:
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 6425f5f838e0..660cd15b6228 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -60,6 +60,7 @@
>  #include <crypto/blake2s.h>
>  #ifdef CONFIG_VDSO_GETRANDOM
>  #include <vdso/getrandom.h>
> +#include <vdso/datapage.h>
>  #endif
>  #include <asm/archrandom.h>
>  #include <asm/processor.h>
> @@ -407,6 +408,9 @@ static void crng_reseed(struct work_struct *work)
>	/*
>	 * We copy the new key into the base_crng, overwriting the old one,
>	 * and update the generation counter. We avoid hitting ULONG_MAX,
>	 * because the per-cpu crngs are initialized to ULONG_MAX, so this
>	 * forces new CPUs that come online to always initialize.
>	 */
>	spin_lock_irqsave(&base_crng.lock, flags);
>	memcpy(base_crng.key, key, sizeof(base_crng.key));
>	next_gen = base_crng.generation + 1;
>  	if (next_gen == ULONG_MAX)
>  		++next_gen;
>  	WRITE_ONCE(base_crng.generation, next_gen);
> +#ifdef CONFIG_VDSO_GETRANDOM
> +	smp_store_release(&_vdso_rng_data.generation, next_gen + 1);
> +#endif

It's confusing that "uninitialized generation" is ULONG_MAX in the per-cpu
crngs, but 0 in the vdso_rng_data.  That results in a weird off-by one thing,
where the vdso_rng_data generation number has to be 1 higher.

Would it be possible to use 0 for both?

- Eric