2024-01-22 17:10:34

by Michael Kelley

[permalink] [raw]
Subject: [PATCH 1/1] x86/hyperv: Use Hyper-V entropy to seed guest random number generator

From: Michael Kelley <[email protected]>

A Hyper-V host provides its guest VMs with entropy in a custom ACPI
table named "OEM0". The entropy bits are updated each time Hyper-V
boots the VM, and are suitable for seeding the Linux guest random
number generator (rng). See a brief description of OEM0 in [1].

Generation 2 VMs on Hyper-V boot using UEFI. Existing EFI code in
Linux seeds the rng with entropy bits from the EFI_RNG_PROTOCOL.
Via this path, the rng is seeded very early during boot with good
entropy. The ACPI OEM0 table is still provided in such VMs, though
it isn't needed.

But Generation 1 VMs on Hyper-V boot from BIOS. For these VMs, Linux
doesn't currently get any entropy from the Hyper-V host. While this
is not fundamentally broken because Linux can generate its own entropy,
using the Hyper-V host provided entropy would get the rng off to a
better start and would do so earlier in the boot process.

Improve the rng seeding for Generation 1 VMs by having Hyper-V specific
code in Linux take advantage of the OEM0 table to seed the rng. Because
the OEM0 table is custom to Hyper-V, parse it directly in the Hyper-V
code in the Linux kernel and use add_bootloader_randomness() to
seed the rng. Once the entropy bits are read from OEM0, zero them
out in the table so they don't appear in /sys/firmware/acpi/tables/OEM0
in the running VM.

An equivalent change is *not* made for Linux VMs on Hyper-V for
ARM64. Such VMs are always Generation 2 and the rng is seeded
with entropy obtained via the EFI_RNG_PROTOCOL as described above.

[1] https://download.microsoft.com/download/1/c/9/1c9813b8-089c-4fef-b2ad-ad80e79403ba/Whitepaper%20-%20The%20Windows%2010%20random%20number%20generation%20infrastructure.pdf

Signed-off-by: Michael Kelley <[email protected]>
---
arch/x86/kernel/cpu/mshyperv.c | 1 +
drivers/hv/hv_common.c | 62 ++++++++++++++++++++++++++++++++++
include/asm-generic/mshyperv.h | 2 ++
3 files changed, 65 insertions(+)

diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index e6bba12c759c..c202a60ecc6c 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -640,6 +640,7 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
.init.x2apic_available = ms_hyperv_x2apic_available,
.init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
.init.init_platform = ms_hyperv_init_platform,
+ .init.guest_late_init = ms_hyperv_late_init,
#ifdef CONFIG_AMD_MEM_ENCRYPT
.runtime.sev_es_hcall_prepare = hv_sev_es_hcall_prepare,
.runtime.sev_es_hcall_finish = hv_sev_es_hcall_finish,
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index ccad7bca3fd3..ebae19b708b4 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -20,6 +20,8 @@
#include <linux/sched/task_stack.h>
#include <linux/panic_notifier.h>
#include <linux/ptrace.h>
+#include <linux/random.h>
+#include <linux/efi.h>
#include <linux/kdebug.h>
#include <linux/kmsg_dump.h>
#include <linux/slab.h>
@@ -348,6 +350,66 @@ int __init hv_common_init(void)
return 0;
}

+void __init ms_hyperv_late_init(void)
+{
+ struct acpi_table_header *header;
+ acpi_status status;
+ u8 *randomdata;
+ u32 length, i;
+
+ /*
+ * Seed the Linux random number generator with entropy provided by
+ * the Hyper-V host in ACPI table OEM0. It would be nice to do this
+ * even earlier in ms_hyperv_init_platform(), but the ACPI subsystem
+ * isn't set up at that point. Skip if booted via EFI as generic EFI
+ * code has already done some seeding using the EFI RNG protocol.
+ */
+ if (!IS_ENABLED(CONFIG_ACPI) || efi_enabled(EFI_BOOT))
+ return;
+
+ status = acpi_get_table("OEM0", 0, &header);
+ if (ACPI_FAILURE(status) || !header) {
+ pr_info("Hyper-V: ACPI table OEM0 not found\n");
+ return;
+ }
+
+ /*
+ * Since the "OEM0" table name is for OEM specific usage, verify
+ * that what we're seeing purports to be from Microsoft.
+ */
+ if (strncmp(header->oem_table_id, "MICROSFT", 8))
+ goto error;
+
+ /*
+ * Ensure the length is reasonable. Requiring at least 32 bytes and
+ * no more than 256 bytes is somewhat arbitrary. Hyper-V currently
+ * provides 64 bytes, but allow for a change in a later version.
+ */
+ if (header->length < sizeof(*header) + 32 ||
+ header->length > sizeof(*header) + 256)
+ goto error;
+
+ length = header->length - sizeof(*header);
+ randomdata = (u8 *)(header + 1);
+ add_bootloader_randomness(randomdata, length);
+
+ /*
+ * To prevent the seed data from being visible in /sys/firmware/acpi,
+ * zero out the random data in the ACPI table and fixup the checksum.
+ */
+ for (i = 0; i < length; i++) {
+ header->checksum += randomdata[i];
+ randomdata[i] = 0;
+ }
+
+ acpi_put_table(header);
+ return;
+
+error:
+ pr_info("Hyper-V: Ignoring malformed ACPI table OEM0\n");
+ acpi_put_table(header);
+}
+
/*
* Hyper-V specific initialization and die code for
* individual CPUs that is common across all architectures.
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index 430f0ae0dde2..e861223093df 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -193,6 +193,7 @@ extern u64 (*hv_read_reference_counter)(void);

int __init hv_common_init(void);
void __init hv_common_free(void);
+void __init ms_hyperv_late_init(void);
int hv_common_cpu_init(unsigned int cpu);
int hv_common_cpu_die(unsigned int cpu);

@@ -290,6 +291,7 @@ void hv_setup_dma_ops(struct device *dev, bool coherent);
static inline bool hv_is_hyperv_initialized(void) { return false; }
static inline bool hv_is_hibernation_supported(void) { return false; }
static inline void hyperv_cleanup(void) {}
+static inline void ms_hyperv_late_init(void) {}
static inline bool hv_is_isolation_supported(void) { return false; }
static inline enum hv_isolation_type hv_get_isolation_type(void)
{
--
2.25.1



2024-03-04 06:57:33

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/hyperv: Use Hyper-V entropy to seed guest random number generator

On Mon, Jan 22, 2024 at 08:00:03AM -0800, [email protected] wrote:
> From: Michael Kelley <[email protected]>
>
> A Hyper-V host provides its guest VMs with entropy in a custom ACPI
> table named "OEM0". The entropy bits are updated each time Hyper-V
> boots the VM, and are suitable for seeding the Linux guest random
> number generator (rng). See a brief description of OEM0 in [1].
>
> Generation 2 VMs on Hyper-V boot using UEFI. Existing EFI code in

Using -> use, I think.

> Linux seeds the rng with entropy bits from the EFI_RNG_PROTOCOL.
> Via this path, the rng is seeded very early during boot with good
> entropy. The ACPI OEM0 table is still provided in such VMs, though
> it isn't needed.
>
> But Generation 1 VMs on Hyper-V boot from BIOS. For these VMs, Linux
> doesn't currently get any entropy from the Hyper-V host. While this
> is not fundamentally broken because Linux can generate its own entropy,
> using the Hyper-V host provided entropy would get the rng off to a
> better start and would do so earlier in the boot process.

I think is a good idea.

>
> Improve the rng seeding for Generation 1 VMs by having Hyper-V specific
> code in Linux take advantage of the OEM0 table to seed the rng. Because
> the OEM0 table is custom to Hyper-V, parse it directly in the Hyper-V
> code in the Linux kernel and use add_bootloader_randomness() to
> seed the rng. Once the entropy bits are read from OEM0, zero them
> out in the table so they don't appear in /sys/firmware/acpi/tables/OEM0
> in the running VM.
>
> An equivalent change is *not* made for Linux VMs on Hyper-V for
> ARM64. Such VMs are always Generation 2 and the rng is seeded
> with entropy obtained via the EFI_RNG_PROTOCOL as described above.
>
> [1] https://download.microsoft.com/download/1/c/9/1c9813b8-089c-4fef-b2ad-ad80e79403ba/Whitepaper%20-%20The%20Windows%2010%20random%20number%20generation%20infrastructure.pdf
>
> Signed-off-by: Michael Kelley <[email protected]>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 1 +
> drivers/hv/hv_common.c | 62 ++++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 2 ++
> 3 files changed, 65 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index e6bba12c759c..c202a60ecc6c 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -640,6 +640,7 @@ const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
> .init.x2apic_available = ms_hyperv_x2apic_available,
> .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
> .init.init_platform = ms_hyperv_init_platform,
> + .init.guest_late_init = ms_hyperv_late_init,
> #ifdef CONFIG_AMD_MEM_ENCRYPT
> .runtime.sev_es_hcall_prepare = hv_sev_es_hcall_prepare,
> .runtime.sev_es_hcall_finish = hv_sev_es_hcall_finish,
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index ccad7bca3fd3..ebae19b708b4 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -20,6 +20,8 @@
> #include <linux/sched/task_stack.h>
> #include <linux/panic_notifier.h>
> #include <linux/ptrace.h>
> +#include <linux/random.h>
> +#include <linux/efi.h>
> #include <linux/kdebug.h>
> #include <linux/kmsg_dump.h>
> #include <linux/slab.h>
> @@ -348,6 +350,66 @@ int __init hv_common_init(void)
> return 0;
> }
>
> +void __init ms_hyperv_late_init(void)
> +{
> + struct acpi_table_header *header;
> + acpi_status status;
> + u8 *randomdata;
> + u32 length, i;
> +
> + /*
> + * Seed the Linux random number generator with entropy provided by
> + * the Hyper-V host in ACPI table OEM0. It would be nice to do this
> + * even earlier in ms_hyperv_init_platform(), but the ACPI subsystem
> + * isn't set up at that point. Skip if booted via EFI as generic EFI
> + * code has already done some seeding using the EFI RNG protocol.
> + */
> + if (!IS_ENABLED(CONFIG_ACPI) || efi_enabled(EFI_BOOT))
> + return;
> +
> + status = acpi_get_table("OEM0", 0, &header);
> + if (ACPI_FAILURE(status) || !header) {
> + pr_info("Hyper-V: ACPI table OEM0 not found\n");

I would like this to be a pr_debug() instead of pr_info(), considering
using the negative case may cause users to think not having this table
can be problematic.

Alternatively, we can remove this message here, and then ...

> + return;
> + }
> +

.. add a pr_debug() here to indicate that the table was found.

pr_info("Hyper-V: Seeding randomness with data from ACPI table OEM0\n");

Dexuan, Saurabh, Haiyang and Long, can you give an ack or nack to this
patch and help test it?

Thanks,
Wei.

> + /*
> + * Since the "OEM0" table name is for OEM specific usage, verify
> + * that what we're seeing purports to be from Microsoft.
> + */
> + if (strncmp(header->oem_table_id, "MICROSFT", 8))
> + goto error;
> +
> + /*
> + * Ensure the length is reasonable. Requiring at least 32 bytes and
> + * no more than 256 bytes is somewhat arbitrary. Hyper-V currently
> + * provides 64 bytes, but allow for a change in a later version.
> + */
> + if (header->length < sizeof(*header) + 32 ||
> + header->length > sizeof(*header) + 256)
> + goto error;
> +
> + length = header->length - sizeof(*header);
> + randomdata = (u8 *)(header + 1);
> + add_bootloader_randomness(randomdata, length);
> +
> + /*
> + * To prevent the seed data from being visible in /sys/firmware/acpi,
> + * zero out the random data in the ACPI table and fixup the checksum.
> + */
> + for (i = 0; i < length; i++) {
> + header->checksum += randomdata[i];
> + randomdata[i] = 0;
> + }
> +
> + acpi_put_table(header);
> + return;
> +
> +error:
> + pr_info("Hyper-V: Ignoring malformed ACPI table OEM0\n");
> + acpi_put_table(header);
> +}
> +
> /*
> * Hyper-V specific initialization and die code for
> * individual CPUs that is common across all architectures.
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 430f0ae0dde2..e861223093df 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -193,6 +193,7 @@ extern u64 (*hv_read_reference_counter)(void);
>
> int __init hv_common_init(void);
> void __init hv_common_free(void);
> +void __init ms_hyperv_late_init(void);
> int hv_common_cpu_init(unsigned int cpu);
> int hv_common_cpu_die(unsigned int cpu);
>
> @@ -290,6 +291,7 @@ void hv_setup_dma_ops(struct device *dev, bool coherent);
> static inline bool hv_is_hyperv_initialized(void) { return false; }
> static inline bool hv_is_hibernation_supported(void) { return false; }
> static inline void hyperv_cleanup(void) {}
> +static inline void ms_hyperv_late_init(void) {}
> static inline bool hv_is_isolation_supported(void) { return false; }
> static inline enum hv_isolation_type hv_get_isolation_type(void)
> {
> --
> 2.25.1
>

2024-03-06 17:43:55

by Michael Kelley

[permalink] [raw]
Subject: RE: [PATCH 1/1] x86/hyperv: Use Hyper-V entropy to seed guest random number generator

From: [email protected] @ 2024-03-04 6:57 UTC
>
> > +void __init ms_hyperv_late_init(void)
> > +{
> > + struct acpi_table_header *header;
> > + acpi_status status;
> > + u8 *randomdata;
> > + u32 length, i;
> > +
> > + /*
> > + * Seed the Linux random number generator with entropy provided by
> > + * the Hyper-V host in ACPI table OEM0. It would be nice to do this
> > + * even earlier in ms_hyperv_init_platform(), but the ACPI subsystem
> > + * isn't set up at that point. Skip if booted via EFI as generic EFI
> > + * code has already done some seeding using the EFI RNG protocol.
> > + */
> > + if (!IS_ENABLED(CONFIG_ACPI) || efi_enabled(EFI_BOOT))
> > + return;
> > +
> > + status = acpi_get_table("OEM0", 0, &header);
> > + if (ACPI_FAILURE(status) || !header) {
> > + pr_info("Hyper-V: ACPI table OEM0 not found\n");
>
> I would like this to be a pr_debug() instead of pr_info(), considering
> using the negative case may cause users to think not having this table
> can be problematic.
>
> Alternatively, we can remove this message here, and then ...
>
> > + return;
> > + }
> > +
>
> ... add a pr_debug() here to indicate that the table was found.
>
> pr_info("Hyper-V: Seeding randomness with data from ACPI table OEM0\n");

You wrote the code as "pr_info()" but your comment suggests "pr_debug()".
I'm assuming pr_debug() is better because we don't really need any output
on success or failure. If trying to debug something related to the rng,
even with no explicit output it's relatively easy to tell whether a Gen1 VM
picked up any entropy from the OEM0 table. When it does, this dmesg
line will appear much earlier than when it does not.

[ 0.000000] random: crng init done

I'll spin a v2 with this tweak and your wording comment on the
commit message.

Michael

>
> Dexuan, Saurabh, Haiyang and Long, can you give an ack or nack to this
> patch and help test it?
>
> Thanks,
> Wei.

2024-03-08 23:53:19

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH 1/1] x86/hyperv: Use Hyper-V entropy to seed guest random number generator

On Wed, Mar 06, 2024 at 05:43:41PM +0000, Michael Kelley wrote:
> From: [email protected] @ 2024-03-04 6:57 UTC
> >
> > > +void __init ms_hyperv_late_init(void)
> > > +{
> > > + struct acpi_table_header *header;
> > > + acpi_status status;
> > > + u8 *randomdata;
> > > + u32 length, i;
> > > +
> > > + /*
> > > + * Seed the Linux random number generator with entropy provided by
> > > + * the Hyper-V host in ACPI table OEM0. It would be nice to do this
> > > + * even earlier in ms_hyperv_init_platform(), but the ACPI subsystem
> > > + * isn't set up at that point. Skip if booted via EFI as generic EFI
> > > + * code has already done some seeding using the EFI RNG protocol.
> > > + */
> > > + if (!IS_ENABLED(CONFIG_ACPI) || efi_enabled(EFI_BOOT))
> > > + return;
> > > +
> > > + status = acpi_get_table("OEM0", 0, &header);
> > > + if (ACPI_FAILURE(status) || !header) {
> > > + pr_info("Hyper-V: ACPI table OEM0 not found\n");
> >
> > I would like this to be a pr_debug() instead of pr_info(), considering
> > using the negative case may cause users to think not having this table
> > can be problematic.
> >
> > Alternatively, we can remove this message here, and then ...
> >
> > > + return;
> > > + }
> > > +
> >
> > ... add a pr_debug() here to indicate that the table was found.
> >
> > pr_info("Hyper-V: Seeding randomness with data from ACPI table OEM0\n");
>
> You wrote the code as "pr_info()" but your comment suggests "pr_debug()".
> I'm assuming pr_debug() is better because we don't really need any output

Yes, I meant to use pr_debug() here. Sorry for the confusion. The
pr_info() was a c&p error.

Thanks,
Wei.