Re: [PATCH v4] random: fix crash on multiple early calls to add_bootloader_randomness()

[Jason A. Donenfeld]

On 12/3/21 16:39, Jason A. Donenfeld wrote:
> Hi Dominik,
>
> Thanks for your analysis. Some more questions:
>
> On Fri, Dec 3, 2021 at 8:59 AM Dominik Brodowski
> <linux@xxxxxxxxxxxxxxxxxxxx> wrote:
> > On subsequent calls to add_bootloader_randomness() and then to
> > add_hwgenerator_randomness(), crng_fast_load() will be skipped. Instead,
> > wait_event_interruptible() (which makes no sense for the init process)
> > and then credit_entropy_bits() will be called. If the entropy count for
> > that second seed is large enough, that proceeds to crng_reseed().
> > However, crng_reseed() may depend on workqueues being available, which
> > is not the case early during boot.
>
> It sounds like *the* issue you've identified is that crng_reseed()
> calls into workqueue functions too early in init, right? The bug is
> about paths into crng_reseed() that might cause that?
>
> If so, then specifically, are you referring to crng_reseed()'s call to
> numa_crng_init()? In other words, the cause of the bug would be
> 6c1e851c4edc ("random: fix possible sleeping allocation from irq
> context")? If that's the case, then I wonder if the problem you're
> seeing goes away if you revert both 6c1e851c4edc ("random: fix
> possible sleeping allocation from irq context") and its primary
> predecessor, 8ef35c866f88 ("random: set up the NUMA crng instances
> after the CRNG is fully initialized"). These fix an actual bug, so I'm
> not suggesting we actually revert these in the tree, but for the
> purpose of testing, I'm wondering if this is actually the root cause
> of the bug you're seeing.

If the above holds, I wonder if something more basic like the below 
would do the trick -- deferring the bringup of the secondary pools until 
later on in rand_initialize.

diff --git a/drivers/char/random.c b/drivers/char/random.c
index c81485e2f126..e71b34bf9a2a 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -832,7 +832,6 @@ static void __init crng_initialize_primary(struct 
crng_state *crng)
 	_extract_entropy(&input_pool, &crng->state[4], sizeof(__u32) * 12, 0);
 	if (crng_init_try_arch_early(crng) && trust_cpu) {
 		invalidate_batched_entropy();
-		numa_crng_init();
 		crng_init = 2;
 		pr_notice("crng done (trusting CPU's manufacturer)\n");
 	}
@@ -840,13 +839,13 @@ static void __init crng_initialize_primary(struct 
crng_state *crng)
 }

 #ifdef CONFIG_NUMA
-static void do_numa_crng_init(struct work_struct *work)
+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);
+	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);
@@ -861,13 +860,6 @@ static void do_numa_crng_init(struct work_struct *work)
 		kfree(pool);
 	}
 }
-
-static DECLARE_WORK(numa_crng_init_work, do_numa_crng_init);
-
-static void numa_crng_init(void)
-{
-	schedule_work(&numa_crng_init_work);
-}
 #else
 static void numa_crng_init(void) {}
 #endif
@@ -977,7 +969,6 @@ static void crng_reseed(struct crng_state *crng, 
struct entropy_store *r)
 	spin_unlock_irqrestore(&crng->lock, flags);
 	if (crng == &primary_crng && crng_init < 2) {
 		invalidate_batched_entropy();
-		numa_crng_init();
 		crng_init = 2;
 		process_random_ready_list();
 		wake_up_interruptible(&crng_init_wait);
@@ -1787,6 +1778,7 @@ int __init rand_initialize(void)
 {
 	init_std_data(&input_pool);
 	crng_initialize_primary(&primary_crng);
+	numa_crng_init();
 	crng_global_init_time = jiffies;
 	if (ratelimit_disable) {
 		urandom_warning.interval = 0;