[PATCH] random: defer use of bootloader randomness to random_init()

From: Jason A. Donenfeld
Date: Tue Jun 07 2022 - 07:15:40 EST

Next message: Krzysztof Kozlowski: "Re: [PATCH 1/4] dt-bindings: arm: qcom: document qcom,msm-id and qcom,board-id"
Previous message: Marc Kleine-Budde: "Re: [RFC PATCH 04/13] can: slcan: use CAN network device driver API"
Next in thread: Jason A. Donenfeld: "Re: [PATCH] random: defer use of bootloader randomness to random_init()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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 changes
the name add_bootloader_randomness() to register_bootloader_seed(), and
insists that its buffer remain alive during all of __init.

A potential downside is that EFI runs a little bit before random_init(),
which means we were getting that seed a lot earlier before.

Fixes: f5bda35fba61 ("random: use static branch for crng_ready()")
Reported-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
Cc: Ard Biesheuvel <ardb@xxxxxxxxxx>
Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
Cc: Russell King <linux@xxxxxxxxxxxxxxx>
Cc: Arnd Bergmann <arnd@xxxxxxxx>
Cc: Phil Elwell <phil@xxxxxxxxxxxxxxx>
Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
---
Ard - I guess we've got two things to figure out here:

1) How much we gain by the situation before this patch of EFI supplying
its seed before random_init().
2) Whether those buffers are guaranteed to live throughout __init, or if
this patch introduces a UaF.

-Jason

drivers/char/random.c | 38 ++++++++++++++++++--------------------
drivers/firmware/efi/efi.c | 2 +-
drivers/of/fdt.c | 2 +-
include/linux/random.h | 2 +-
4 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 4862d4d3ec49..d9d00143c7c5 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -678,7 +678,6 @@ static void __cold _credit_init_bits(size_t bits)
*
* void add_device_randomness(const void *buf, size_t len);
* void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy);
- * void add_bootloader_randomness(const void *buf, size_t len);
* void add_vmfork_randomness(const void *unique_vm_id, size_t len);
* void add_interrupt_randomness(int irq);
* void add_input_randomness(unsigned int type, unsigned int code, unsigned int value);
@@ -696,10 +695,6 @@ static void __cold _credit_init_bits(size_t bits)
* entropy as specified by the caller. If the entropy pool is full it will
* block until more entropy is needed.
*
- * add_bootloader_randomness() is called by bootloader drivers, such as EFI
- * and device tree, and credits its input depending on whether or not the
- * configuration option CONFIG_RANDOM_TRUST_BOOTLOADER is set.
- *
* add_vmfork_randomness() adds a unique (but not necessarily secret) ID
* representing the current instance of a VM to the pool, without crediting,
* and then force-reseeds the crng so that it takes effect immediately.
@@ -765,6 +760,18 @@ static int random_pm_notification(struct notifier_block *nb, unsigned long actio

static struct notifier_block pm_notifier = { .notifier_call = random_pm_notification };

+/*
+ * Handle random seed passed by bootloader. The buf pointer
+ * must remain alive during the kernel's init sequence.
+ */
+static __initdata const void *bootloader_seed;
+static __initdata size_t bootloader_seed_bytes;
+void __init register_bootloader_seed(const void *buf, size_t len)
+{
+ bootloader_seed = buf;
+ bootloader_seed_bytes = len;
+}
+
/*
* The first collection of entropy occurs at system boot while interrupts
* are still turned off. Here we push in latent entropy, RDSEED, a timestamp,
@@ -793,6 +800,8 @@ int __init random_init(const char *command_line)
}
_mix_pool_bytes(&entropy, sizeof(entropy));
}
+ if (bootloader_seed && bootloader_seed_bytes)
+ _mix_pool_bytes(bootloader_seed, bootloader_seed_bytes);
_mix_pool_bytes(&now, sizeof(now));
_mix_pool_bytes(utsname(), sizeof(*(utsname())));
_mix_pool_bytes(command_line, strlen(command_line));
@@ -800,9 +809,10 @@ int __init random_init(const char *command_line)

if (crng_ready())
crng_reseed();
- else if (trust_cpu)
- _credit_init_bits(arch_bytes * 8);
- used_arch_random = arch_bytes * 8 >= POOL_READY_BITS;
+ else
+ _credit_init_bits((trust_cpu ? arch_bytes * 8 : 0) +
+ (trust_bootloader ? bootloader_seed_bytes * 8 : 0));
+ used_arch_random = arch_bytes * 8 + bootloader_seed_bytes * 8 >= POOL_READY_BITS;

WARN_ON(register_pm_notifier(&pm_notifier));

@@ -861,18 +871,6 @@ void add_hwgenerator_randomness(const void *buf, size_t len, size_t entropy)
}
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)
-{
- mix_pool_bytes(buf, len);
- if (trust_bootloader)
- credit_init_bits(len * 8);
-}
-EXPORT_SYMBOL_GPL(add_bootloader_randomness);
-
#if IS_ENABLED(CONFIG_VMGENID)
static BLOCKING_NOTIFIER_HEAD(vmfork_chain);

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 860534bcfdac..daf374983012 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -614,7 +614,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
sizeof(*seed) + size);
if (seed != NULL) {
pr_notice("seeding entropy pool\n");
- add_bootloader_randomness(seed->bits, size);
+ register_bootloader_seed(seed->bits, size);
early_memunmap(seed, sizeof(*seed) + size);
} else {
pr_err("Could not map UEFI random seed!\n");
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index a8f5b6532165..389ef0f781c2 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -1201,7 +1201,7 @@ int __init early_init_dt_scan_chosen(char *cmdline)

rng_seed = of_get_flat_dt_prop(node, "rng-seed", &l);
if (rng_seed && l > 0) {
- add_bootloader_randomness(rng_seed, l);
+ register_bootloader_seed(rng_seed, l);

/* try to clear seed so it won't be found. */
fdt_nop_property(initial_boot_params, node, "rng-seed");
diff --git a/include/linux/random.h b/include/linux/random.h
index fae0c84027fd..c4fa7ca008df 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -13,7 +13,6 @@
struct notifier_block;

void add_device_randomness(const void *buf, size_t len);
-void 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;
@@ -72,6 +71,7 @@ static inline unsigned long get_random_canary(void)
return get_random_long() & CANARY_MASK;
}

+void __init register_bootloader_seed(const void *buf, size_t len);
int __init random_init(const char *command_line);
bool rng_is_initialized(void);
bool rng_has_arch_random(void);
--
2.35.1

Next message: Krzysztof Kozlowski: "Re: [PATCH 1/4] dt-bindings: arm: qcom: document qcom,msm-id and qcom,board-id"
Previous message: Marc Kleine-Budde: "Re: [RFC PATCH 04/13] can: slcan: use CAN network device driver API"
Next in thread: Jason A. Donenfeld: "Re: [PATCH] random: defer use of bootloader randomness to random_init()"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]