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;
