Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 90ceecda authored by Eric Biggers's avatar Eric Biggers Committed by Greg Kroah-Hartman
Browse files

random: fix data race on crng_node_pool 

commit 5d73d1e320c3fd94ea15ba5f79301da9a8bcc7de upstream.

extract_crng() and crng_backtrack_protect() load crng_node_pool with a
plain load, which causes undefined behavior if do_numa_crng_init()
modifies it concurrently.

Fix this by using READ_ONCE().  Note: as per the previous discussion
https://lore.kernel.org/lkml/20211219025139.31085-1-ebiggers@kernel.org/T/#u

,
READ_ONCE() is believed to be sufficient here, and it was requested that
it be used here instead of smp_load_acquire().

Also change do_numa_crng_init() to set crng_node_pool using
cmpxchg_release() instead of mb() + cmpxchg(), as the former is
sufficient here but is more lightweight.

Fixes: 1e7f583a ("random: make /dev/urandom scalable for silly userspace programs")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarEric Biggers <ebiggers@google.com>
Acked-by: default avatarPaul E. McKenney <paulmck@kernel.org>
Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent a4fa4377

drivers/char/random.c

+22 −20
Original line number Diff line number Diff line
@@ -904,8 +904,8 @@ static void do_numa_crng_init(struct work_struct *work) 
		crng_initialize(crng);

		pool[i] = crng;

	}

	mb();

	if (cmpxchg(&crng_node_pool, NULL, pool)) {

	/* pairs with READ_ONCE() in select_crng() */

	if (cmpxchg_release(&crng_node_pool, NULL, pool) != NULL) {

		for_each_node(i)

			kfree(pool[i]);

		kfree(pool);
@@ -918,8 +918,26 @@ static void numa_crng_init(void) 
{

	schedule_work(&numa_crng_init_work);

}



static struct crng_state *select_crng(void)

{

	struct crng_state **pool;

	int nid = numa_node_id();



	/* pairs with cmpxchg_release() in do_numa_crng_init() */

	pool = READ_ONCE(crng_node_pool);

	if (pool && pool[nid])

		return pool[nid];



	return &primary_crng;

}

#else

static void numa_crng_init(void) {}



static struct crng_state *select_crng(void)

{

	return &primary_crng;

}

#endif



/*
@@ -1068,15 +1086,7 @@ static void _extract_crng(struct crng_state *crng, 


static void extract_crng(__u8 out[CHACHA_BLOCK_SIZE])

{

	struct crng_state *crng = NULL;



#ifdef CONFIG_NUMA

	if (crng_node_pool)

		crng = crng_node_pool[numa_node_id()];

	if (crng == NULL)

#endif

		crng = &primary_crng;

	_extract_crng(crng, out);

	_extract_crng(select_crng(), out);

}



/*
@@ -1105,15 +1115,7 @@ static void _crng_backtrack_protect(struct crng_state *crng, 


static void crng_backtrack_protect(__u8 tmp[CHACHA_BLOCK_SIZE], int used)

{

	struct crng_state *crng = NULL;



#ifdef CONFIG_NUMA

	if (crng_node_pool)

		crng = crng_node_pool[numa_node_id()];

	if (crng == NULL)

#endif

		crng = &primary_crng;

	_crng_backtrack_protect(crng, tmp, used);

	_crng_backtrack_protect(select_crng(), tmp, used);

}



static ssize_t extract_crng_user(void __user *buf, size_t nbytes)