On Sun, 2018-04-22 at 15:53 +0200, Greg Kroah-Hartman wrote:
> 4.9-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Theodore Ts'o <tytso@xxxxxxx>
>
> commit 8ef35c866f8862df074a49a93b0309725812dea8 upstream.
>
> Until the primary_crng is fully initialized, don't initialize the NUMA
> crng nodes. Otherwise users of /dev/urandom on NUMA systems before
> the CRNG is fully initialized can get very bad quality randomness. Of
> course everyone should move to getrandom(2) where this won't be an
> issue, but there's a lot of legacy code out there. This related to
> CVE-2018-1108.
>
> Reported-by: Jann Horn <jannh@xxxxxxxxxx>
> Fixes: 1e7f583af67b ("random: make /dev/urandom scalable for silly...")
> Cc: stable@xxxxxxxxxx # 4.8+
> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
In 4.9 (and probably older branches too) this leads to a deadlock:
crng_reseed(primary_crng, ...) takes primary_crng.lock
-> numa_rcng_init()
-> crng_initialize()
-> get_random_bytes()
-> extract_crng()
-> _extract_crng(primary_crng, ...) tries to take primary_crng.lock
I think this can be fixed by backporting commit 4a072c71f49b
"random: silence compiler warnings and fix race" but I'm not sure
whether that depends on other changes.
Ben.
> ---
> drivers/char/random.c | 46 +++++++++++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 19 deletions(-)
>
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -818,6 +818,32 @@ static int crng_fast_load(const char *cp
> return 1;
> }
>
> +#ifdef CONFIG_NUMA
> +static void numa_crng_init(void)
> +{
> + int i;
> + struct crng_state *crng;
> + struct crng_state **pool;
> +
> + pool = kcalloc(nr_node_ids, sizeof(*pool), GFP_KERNEL|__GFP_NOFAIL);
> + for_each_online_node(i) {
> + crng = kmalloc_node(sizeof(struct crng_state),
> + GFP_KERNEL | __GFP_NOFAIL, i);
> + spin_lock_init(&crng->lock);
> + crng_initialize(crng);
> + pool[i] = crng;
> + }
> + mb();
> + if (cmpxchg(&crng_node_pool, NULL, pool)) {
> + for_each_node(i)
> + kfree(pool[i]);
> + kfree(pool);
> + }
> +}
> +#else
> +static void numa_crng_init(void) {}
> +#endif
> +
> static void crng_reseed(struct crng_state *crng, struct entropy_store *r)
> {
> unsigned long flags;
> @@ -847,6 +873,7 @@ static void crng_reseed(struct crng_stat
> memzero_explicit(&buf, sizeof(buf));
> crng->init_time = jiffies;
> if (crng == &primary_crng && crng_init < 2) {
> + numa_crng_init();
> crng_init = 2;
> process_random_ready_list();
> wake_up_interruptible(&crng_init_wait);
> @@ -1659,28 +1686,9 @@ static void init_std_data(struct entropy
> */
> static int rand_initialize(void)
> {
> -#ifdef CONFIG_NUMA
> - int i;
> - struct crng_state *crng;
> - struct crng_state **pool;
> -#endif
> -
> init_std_data(&input_pool);
> init_std_data(&blocking_pool);
> crng_initialize(&primary_crng);
> -
> -#ifdef CONFIG_NUMA
> - pool = kcalloc(nr_node_ids, sizeof(*pool), GFP_KERNEL|__GFP_NOFAIL);
> - for_each_online_node(i) {
> - crng = kmalloc_node(sizeof(struct crng_state),
> - GFP_KERNEL | __GFP_NOFAIL, i);
> - spin_lock_init(&crng->lock);
> - crng_initialize(crng);
> - pool[i] = crng;
> - }
> - mb();
> - crng_node_pool = pool;
> -#endif
> return 0;
> }
> early_initcall(rand_initialize);
>
>
--
Ben Hutchings
It is easier to write an incorrect program
than to understand a correct one.
Attachment:
signature.asc
Description: This is a digitally signed message part