| Message ID | 20220607113238.769088-1-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] random: defer crediting bootloader randomness to random_init() | expand |
Hi Jason, On 07/06/2022 12:32, Jason A. Donenfeld wrote: > Stephen reported that a static key warning splat appears during early > boot on systems that credit randomness from device trees that contain an > "rng-seed" property, because because setup_machine_fdt() is called > before jump_label_init() during setup_arch(): > > static_key_enable_cpuslocked(): static key '0xffffffe51c6fcfc0' used before call to jump_label_init() > WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:166 static_key_enable_cpuslocked+0xb0/0xb8 > Modules linked in: > CPU: 0 PID: 0 Comm: swapper Not tainted 5.18.0+ #224 44b43e377bfc84bc99bb5ab885ff694984ee09ff > pstate: 600001c9 (nZCv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : static_key_enable_cpuslocked+0xb0/0xb8 > lr : static_key_enable_cpuslocked+0xb0/0xb8 > sp : ffffffe51c393cf0 > x29: ffffffe51c393cf0 x28: 000000008185054c x27: 00000000f1042f10 > x26: 0000000000000000 x25: 00000000f10302b2 x24: 0000002513200000 > x23: 0000002513200000 x22: ffffffe51c1c9000 x21: fffffffdfdc00000 > x20: ffffffe51c2f0831 x19: ffffffe51c6fcfc0 x18: 00000000ffff1020 > x17: 00000000e1e2ac90 x16: 00000000000000e0 x15: ffffffe51b710708 > x14: 0000000000000066 x13: 0000000000000018 x12: 0000000000000000 > x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000 > x8 : 0000000000000000 x7 : 61632065726f6665 x6 : 6220646573752027 > x5 : ffffffe51c641d25 x4 : ffffffe51c13142c x3 : ffff0a00ffffff05 > x2 : 40000000ffffe003 x1 : 00000000000001c0 x0 : 0000000000000065 > Call trace: > static_key_enable_cpuslocked+0xb0/0xb8 > static_key_enable+0x2c/0x40 > crng_set_ready+0x24/0x30 > execute_in_process_context+0x80/0x90 > _credit_init_bits+0x100/0x154 > add_bootloader_randomness+0x64/0x78 > early_init_dt_scan_chosen+0x140/0x184 > early_init_dt_scan_nodes+0x28/0x4c > early_init_dt_scan+0x40/0x44 > setup_machine_fdt+0x7c/0x120 > setup_arch+0x74/0x1d8 > start_kernel+0x84/0x44c > __primary_switched+0xc0/0xc8 > ---[ end trace 0000000000000000 ]--- > random: crng init done > Machine model: Google Lazor (rev1 - 2) with LTE > > A trivial fix went in to address this on arm64, 73e2d827a501 ("arm64: > Initialize jump labels before setup_machine_fdt()"). But it appears that > fixing it on other platforms might not be so trivial. And in the past > there have been problems related to add_bootloader_randomness() being > called too early in boot for what it needed. > > This patch defers all entropy crediting until random_init(), where we > can be sure that all facilities we need are up and running. It still > mixes the actual seed immediately, so that it's maximally useful, but > the crediting doesn't happen until later. > > This also has the positive effect of allowing rng_has_arch_random() to > reflect bootloader randomness. > > Fixes: f5bda35fba61 ("random: use static branch for crng_ready()") > Reported-by: Stephen Boyd <swboyd@chromium.org> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Phil Elwell <phil@raspberrypi.com> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/char/random.c | 11 ++++++----- > include/linux/random.h | 2 +- > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/random.c b/drivers/char/random.c > index 4862d4d3ec49..34399e4bad19 100644 > --- a/drivers/char/random.c > +++ b/drivers/char/random.c > @@ -725,8 +725,9 @@ static void __cold _credit_init_bits(size_t bits) > **********************************************************************/ > > static bool used_arch_random; > -static bool trust_cpu __ro_after_init = IS_ENABLED(CONFIG_RANDOM_TRUST_CPU); > -static bool trust_bootloader __ro_after_init = IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER); > +static bool trust_cpu __initdata = IS_ENABLED(CONFIG_RANDOM_TRUST_CPU); > +static bool trust_bootloader __initdata = IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER); > +static size_t bootloader_seed_bytes __initdata; > static int __init parse_trust_cpu(char *arg) > { > return kstrtobool(arg, &trust_cpu); > @@ -793,6 +794,7 @@ int __init random_init(const char *command_line) > } > _mix_pool_bytes(&entropy, sizeof(entropy)); > } > + arch_bytes += bootloader_seed_bytes; > _mix_pool_bytes(&now, sizeof(now)); > _mix_pool_bytes(utsname(), sizeof(*(utsname()))); > _mix_pool_bytes(command_line, strlen(command_line)); > @@ -865,13 +867,12 @@ EXPORT_SYMBOL_GPL(add_hwgenerator_randomness); > * Handle random seed passed by bootloader, and credit it if > * CONFIG_RANDOM_TRUST_BOOTLOADER is set. > */ > -void __cold add_bootloader_randomness(const void *buf, size_t len) > +void __init add_bootloader_randomness(const void *buf, size_t len) > { > mix_pool_bytes(buf, len); > if (trust_bootloader) > - credit_init_bits(len * 8); > + bootloader_seed_bytes = len; > } > -EXPORT_SYMBOL_GPL(add_bootloader_randomness); > > #if IS_ENABLED(CONFIG_VMGENID) > static BLOCKING_NOTIFIER_HEAD(vmfork_chain); > diff --git a/include/linux/random.h b/include/linux/random.h > index fae0c84027fd..223b4bd584e7 100644 > --- a/include/linux/random.h > +++ b/include/linux/random.h > @@ -13,7 +13,7 @@ > struct notifier_block; > > void add_device_randomness(const void *buf, size_t len); > -void add_bootloader_randomness(const void *buf, size_t len); > +void __init add_bootloader_randomness(const void *buf, size_t len); > void add_input_randomness(unsigned int type, unsigned int code, > unsigned int value) __latent_entropy; > void add_interrupt_randomness(int irq) __latent_entropy; After a trivial merge conflict (into 5.15) was resolved, this patch allows the downstream kernel with CONFIG_RANDOM_TRUST_BOOTLOADER=y to boot cleanly. However, it does delay the initialisation of crng significantly compared to my hack. With v2: [ 1.981493] bcm2835-rng 3f104000.rng: hwrng registered [ 12.549190] random: crng init done With the hack: [ 0.000000] random: crng init done [ 1.970662] bcm2835-rng 3f104000.rng: hwrng registered I'll leave it to others to decide whether or not this is acceptable. Phil
Hi Phil, On Tue, Jun 07, 2022 at 01:19:41PM +0100, Phil Elwell wrote: > After a trivial merge conflict (into 5.15) was resolved, this patch allows the > downstream kernel with CONFIG_RANDOM_TRUST_BOOTLOADER=y to boot cleanly. > However, it does delay the initialisation of crng significantly compared to my > hack. > > With v2: > [ 1.981493] bcm2835-rng 3f104000.rng: hwrng registered > [ 12.549190] random: crng init done Thanks. It's because the patch is buggy :) v3 coming right up. Jason
diff --git a/drivers/char/random.c b/drivers/char/random.c index 4862d4d3ec49..34399e4bad19 100644 --- a/drivers/char/random.c +++ b/drivers/char/random.c @@ -725,8 +725,9 @@ static void __cold _credit_init_bits(size_t bits) **********************************************************************/ static bool used_arch_random; -static bool trust_cpu __ro_after_init = IS_ENABLED(CONFIG_RANDOM_TRUST_CPU); -static bool trust_bootloader __ro_after_init = IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER); +static bool trust_cpu __initdata = IS_ENABLED(CONFIG_RANDOM_TRUST_CPU); +static bool trust_bootloader __initdata = IS_ENABLED(CONFIG_RANDOM_TRUST_BOOTLOADER); +static size_t bootloader_seed_bytes __initdata; static int __init parse_trust_cpu(char *arg) { return kstrtobool(arg, &trust_cpu); @@ -793,6 +794,7 @@ int __init random_init(const char *command_line) } _mix_pool_bytes(&entropy, sizeof(entropy)); } + arch_bytes += bootloader_seed_bytes; _mix_pool_bytes(&now, sizeof(now)); _mix_pool_bytes(utsname(), sizeof(*(utsname()))); _mix_pool_bytes(command_line, strlen(command_line)); @@ -865,13 +867,12 @@ EXPORT_SYMBOL_GPL(add_hwgenerator_randomness); * Handle random seed passed by bootloader, and credit it if * CONFIG_RANDOM_TRUST_BOOTLOADER is set. */ -void __cold add_bootloader_randomness(const void *buf, size_t len) +void __init add_bootloader_randomness(const void *buf, size_t len) { mix_pool_bytes(buf, len); if (trust_bootloader) - credit_init_bits(len * 8); + bootloader_seed_bytes = len; } -EXPORT_SYMBOL_GPL(add_bootloader_randomness); #if IS_ENABLED(CONFIG_VMGENID) static BLOCKING_NOTIFIER_HEAD(vmfork_chain); diff --git a/include/linux/random.h b/include/linux/random.h index fae0c84027fd..223b4bd584e7 100644 --- a/include/linux/random.h +++ b/include/linux/random.h @@ -13,7 +13,7 @@ struct notifier_block; void add_device_randomness(const void *buf, size_t len); -void add_bootloader_randomness(const void *buf, size_t len); +void __init add_bootloader_randomness(const void *buf, size_t len); void add_input_randomness(unsigned int type, unsigned int code, unsigned int value) __latent_entropy; void add_interrupt_randomness(int irq) __latent_entropy;
Stephen reported that a static key warning splat appears during early boot on systems that credit randomness from device trees that contain an "rng-seed" property, because because setup_machine_fdt() is called before jump_label_init() during setup_arch(): static_key_enable_cpuslocked(): static key '0xffffffe51c6fcfc0' used before call to jump_label_init() WARNING: CPU: 0 PID: 0 at kernel/jump_label.c:166 static_key_enable_cpuslocked+0xb0/0xb8 Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 5.18.0+ #224 44b43e377bfc84bc99bb5ab885ff694984ee09ff pstate: 600001c9 (nZCv dAIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : static_key_enable_cpuslocked+0xb0/0xb8 lr : static_key_enable_cpuslocked+0xb0/0xb8 sp : ffffffe51c393cf0 x29: ffffffe51c393cf0 x28: 000000008185054c x27: 00000000f1042f10 x26: 0000000000000000 x25: 00000000f10302b2 x24: 0000002513200000 x23: 0000002513200000 x22: ffffffe51c1c9000 x21: fffffffdfdc00000 x20: ffffffe51c2f0831 x19: ffffffe51c6fcfc0 x18: 00000000ffff1020 x17: 00000000e1e2ac90 x16: 00000000000000e0 x15: ffffffe51b710708 x14: 0000000000000066 x13: 0000000000000018 x12: 0000000000000000 x11: 0000000000000000 x10: 00000000ffffffff x9 : 0000000000000000 x8 : 0000000000000000 x7 : 61632065726f6665 x6 : 6220646573752027 x5 : ffffffe51c641d25 x4 : ffffffe51c13142c x3 : ffff0a00ffffff05 x2 : 40000000ffffe003 x1 : 00000000000001c0 x0 : 0000000000000065 Call trace: static_key_enable_cpuslocked+0xb0/0xb8 static_key_enable+0x2c/0x40 crng_set_ready+0x24/0x30 execute_in_process_context+0x80/0x90 _credit_init_bits+0x100/0x154 add_bootloader_randomness+0x64/0x78 early_init_dt_scan_chosen+0x140/0x184 early_init_dt_scan_nodes+0x28/0x4c early_init_dt_scan+0x40/0x44 setup_machine_fdt+0x7c/0x120 setup_arch+0x74/0x1d8 start_kernel+0x84/0x44c __primary_switched+0xc0/0xc8 ---[ end trace 0000000000000000 ]--- random: crng init done Machine model: Google Lazor (rev1 - 2) with LTE A trivial fix went in to address this on arm64, 73e2d827a501 ("arm64: Initialize jump labels before setup_machine_fdt()"). But it appears that fixing it on other platforms might not be so trivial. And in the past there have been problems related to add_bootloader_randomness() being called too early in boot for what it needed. This patch defers all entropy crediting until random_init(), where we can be sure that all facilities we need are up and running. It still mixes the actual seed immediately, so that it's maximally useful, but the crediting doesn't happen until later. This also has the positive effect of allowing rng_has_arch_random() to reflect bootloader randomness. Fixes: f5bda35fba61 ("random: use static branch for crng_ready()") Reported-by: Stephen Boyd <swboyd@chromium.org> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Russell King <linux@armlinux.org.uk> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Phil Elwell <phil@raspberrypi.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/char/random.c | 11 ++++++----- include/linux/random.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-)