On Fri, Feb 25, 2022 at 12:24:05PM +0100, Ard Biesheuvel wrote: > On Thu, 24 Feb 2022 at 14:39, Jason A. Donenfeld <Jason@xxxxxxxxx> wrote: > > > > VM Generation ID is a feature from Microsoft, described at > > <https://go.microsoft.com/fwlink/?LinkId=260709>, and supported by > > Hyper-V and QEMU. Its usage is described in Microsoft's RNG whitepaper, > > <https://aka.ms/win10rng>, as: > > > > If the OS is running in a VM, there is a problem that most > > hypervisors can snapshot the state of the machine and later rewind > > the VM state to the saved state. This results in the machine running > > a second time with the exact same RNG state, which leads to serious > > security problems. To reduce the window of vulnerability, Windows > > 10 on a Hyper-V VM will detect when the VM state is reset, retrieve > > a unique (not random) value from the hypervisor, and reseed the root > > RNG with that unique value. This does not eliminate the > > vulnerability, but it greatly reduces the time during which the RNG > > system will produce the same outputs as it did during a previous > > instantiation of the same VM state. > > > > Linux has the same issue, and given that vmgenid is supported already by > > multiple hypervisors, we can implement more or less the same solution. > > So this commit wires up the vmgenid ACPI notification to the RNG's newly > > added add_vmfork_randomness() function. > > > > It can be used from qemu via the `-device vmgenid,guid=auto` parameter. > > After setting that, use `savevm` in the monitor to save the VM state, > > then quit QEMU, start it again, and use `loadvm`. That will trigger this > > driver's notify function, which hands the new UUID to the RNG. This is > > described in <https://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/vmgenid.txt>. > > And there are hooks for this in libvirt as well, described in > > <https://libvirt.org/formatdomain.html#general-metadata>. > > > > Note, however, that the treatment of this as a UUID is considered to be > > an accidental QEMU nuance, per > > <https://github.com/libguestfs/virt-v2v/blob/master/docs/vm-generation-id-across-hypervisors.txt>, > > so this driver simply treats these bytes as an opaque 128-bit binary > > blob, as per the spec. This doesn't really make a difference anyway, > > considering that's how it ends up when handed to the RNG in the end. > > > > This driver builds on prior work from Adrian Catangiu at Amazon, and it > > is my hope that that team can resume maintenance of this driver. > > > > Cc: Adrian Catangiu <adrian@xxxxxxxxx> > > Cc: Laszlo Ersek <lersek@xxxxxxxxxx> > > Cc: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > Cc: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx> > > Cc: Ard Biesheuvel <ardb@xxxxxxxxxx> > > Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx> > > --- > > drivers/virt/Kconfig | 9 +++ > > drivers/virt/Makefile | 1 + > > drivers/virt/vmgenid.c | 121 +++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 131 insertions(+) > > create mode 100644 drivers/virt/vmgenid.c > > > > diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig > > index 8061e8ef449f..d3276dc2095c 100644 > > --- a/drivers/virt/Kconfig > > +++ b/drivers/virt/Kconfig > > drivers/virt does not have a maintainer and this code needs one. > > > @@ -13,6 +13,15 @@ menuconfig VIRT_DRIVERS > > > > if VIRT_DRIVERS > > > > +config VMGENID > > + tristate "Virtual Machine Generation ID driver" > > + default y > > Please make this default m - this code can run as a module and the > feature it relies on is discoverable by udev Or don't supply a default - I don't see why this has any preference. > > + depends on ACPI > > + help > > + Say Y here to use the hypervisor-provided Virtual Machine Generation ID > > + to reseed the RNG when the VM is cloned. This is highly recommended if > > + you intend to do any rollback / cloning / snapshotting of VMs. > > + > > config FSL_HV_MANAGER > > tristate "Freescale hypervisor management driver" > > depends on FSL_SOC > > diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile > > index 3e272ea60cd9..108d0ffcc9aa 100644 > > --- a/drivers/virt/Makefile > > +++ b/drivers/virt/Makefile > > @@ -4,6 +4,7 @@ > > # > > > > obj-$(CONFIG_FSL_HV_MANAGER) += fsl_hypervisor.o > > +obj-$(CONFIG_VMGENID) += vmgenid.o > > obj-y += vboxguest/ > > > > obj-$(CONFIG_NITRO_ENCLAVES) += nitro_enclaves/ > > diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c > > new file mode 100644 > > index 000000000000..5da4dc8f25e3 > > --- /dev/null > > +++ b/drivers/virt/vmgenid.c > > @@ -0,0 +1,121 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Virtual Machine Generation ID driver > > + * > > + * Copyright (C) 2022 Jason A. Donenfeld <Jason@xxxxxxxxx>. All Rights Reserved. > > + * Copyright (C) 2020 Amazon. All rights reserved. > > + * Copyright (C) 2018 Red Hat Inc. All rights reserved. > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/acpi.h> > > +#include <linux/random.h> > > + > > +ACPI_MODULE_NAME("vmgenid"); > > + > > +enum { VMGENID_SIZE = 16 }; > > + > > +static struct { > > + u8 this_id[VMGENID_SIZE]; > > + u8 *next_id; > > +} state; > > + > > This state is singular > > > > +static int vmgenid_acpi_add(struct acpi_device *device) > > +{ > > ... whereas this may be called for multiple instances of the device. > This likely makes no sense, so it is better to reject it here. > > Otherwise, the state should be allocated dynamically. > > > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER }; > > + union acpi_object *pss; > > + phys_addr_t phys_addr; > > + acpi_status status; > > + int ret = 0; > > + > > + if (!device) > > + return -EINVAL; > > + > > + status = acpi_evaluate_object(device->handle, "ADDR", NULL, &buffer); > > + if (ACPI_FAILURE(status)) { > > + ACPI_EXCEPTION((AE_INFO, status, "Evaluating ADDR")); > > + return -ENODEV; > > + } > > + pss = buffer.pointer; > > + if (!pss || pss->type != ACPI_TYPE_PACKAGE || pss->package.count != 2 || > > + pss->package.elements[0].type != ACPI_TYPE_INTEGER || > > + pss->package.elements[1].type != ACPI_TYPE_INTEGER) { > > + ret = -EINVAL; > > + goto out; > > + } > > + > > + phys_addr = (pss->package.elements[0].integer.value << 0) | > > + (pss->package.elements[1].integer.value << 32); > > + state.next_id = acpi_os_map_memory(phys_addr, VMGENID_SIZE); > > No need to use acpi_os_map_memory() here, plain memremap() should be fine. > > > + if (!state.next_id) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + device->driver_data = &state; > > + > > + memcpy(state.this_id, state.next_id, sizeof(state.this_id)); > > + add_device_randomness(state.this_id, sizeof(state.this_id)); > > + > > +out: > > + ACPI_FREE(buffer.pointer); > > + return ret; > > +} > > + > > +static int vmgenid_acpi_remove(struct acpi_device *device) > > +{ > > + if (!device || acpi_driver_data(device) != &state) > > + return -EINVAL; > > + device->driver_data = NULL; > > + if (state.next_id) > > + acpi_os_unmap_memory(state.next_id, VMGENID_SIZE); > > memunmap() here > > > + state.next_id = NULL; > > + return 0; > > +} > > + > > +static void vmgenid_acpi_notify(struct acpi_device *device, u32 event) > > +{ > > + u8 old_id[VMGENID_SIZE]; > > + > > + if (!device || acpi_driver_data(device) != &state) > > + return; > > + memcpy(old_id, state.this_id, sizeof(old_id)); > > + memcpy(state.this_id, state.next_id, sizeof(state.this_id)); > > + if (!memcmp(old_id, state.this_id, sizeof(old_id))) > > + return; > > Is this little dance really necessary? I.e., can we just do > > add_vmfork_randomness(state.next_id, VMGENID_SIZE) > > and be done with it? > > And if we cannot, is it ok to just return without some kind of > diagnostic message? > > > + add_vmfork_randomness(state.this_id, sizeof(state.this_id)); > > +} > > + > > +static const struct acpi_device_id vmgenid_ids[] = { > > + {"VMGENID", 0}, > > + {"QEMUVGID", 0}, > > + { }, > > +}; > > + > > +static struct acpi_driver acpi_driver = { > > + .name = "vm_generation_id", > > + .ids = vmgenid_ids, > > + .owner = THIS_MODULE, > > + .ops = { > > + .add = vmgenid_acpi_add, > > + .remove = vmgenid_acpi_remove, > > + .notify = vmgenid_acpi_notify, > > + } > > +}; > > + > > +static int __init vmgenid_init(void) > > +{ > > + return acpi_bus_register_driver(&acpi_driver); > > +} > > + > > +static void __exit vmgenid_exit(void) > > +{ > > + acpi_bus_unregister_driver(&acpi_driver); > > +} > > + > > +module_init(vmgenid_init); > > +module_exit(vmgenid_exit); > > + > > +MODULE_DEVICE_TABLE(acpi, vmgenid_ids); > > +MODULE_DESCRIPTION("Virtual Machine Generation ID"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 2.35.1 > >
