Messages in this thread Patch in this message | | | Date | Fri, 3 Dec 2021 17:47:41 +0100 | Subject | Re: [PATCH v4] random: fix crash on multiple early calls to add_bootloader_randomness() | From | "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@dominikbrodowski.net> 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;
| |