From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4B837C433F5 for ; Fri, 25 Feb 2022 11:56:44 +0000 (UTC) Received: from localhost ([::1]:39478 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nNZDH-0002YA-AQ for qemu-devel@archiver.kernel.org; Fri, 25 Feb 2022 06:56:43 -0500 Received: from eggs.gnu.org ([209.51.188.92]:45046) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nNZ8s-000615-Dr for qemu-devel@nongnu.org; Fri, 25 Feb 2022 06:52:10 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:40033) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nNZ8n-0003T3-Qh for qemu-devel@nongnu.org; Fri, 25 Feb 2022 06:52:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1645789924; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TczmJXW3cf8MAYhPlLedjQbAu9sWmw7nekowqPG8i94=; b=UBxsCvimpcqwrTpXIfqCoksIqYLkrrreeYE+zT5NClQ0E+/Ox15B010DmLIzeevo2sLvzv z3KftHfCt6clAXO+HAVebUVxnYOfxDdcyYGsLc4uvp5UHj3vHWIsArJbjyW+ErA8pd9DNc TY4qcdURlh+4yk07aKzogrAxWoxSnDQ= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-601-KMwf7A-sPFy6Cpht3Iz56A-1; Fri, 25 Feb 2022 06:52:02 -0500 X-MC-Unique: KMwf7A-sPFy6Cpht3Iz56A-1 Received: by mail-wm1-f70.google.com with SMTP id q127-20020a1ca785000000b0037faac72dcbso2274569wme.1 for ; Fri, 25 Feb 2022 03:52:02 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=TczmJXW3cf8MAYhPlLedjQbAu9sWmw7nekowqPG8i94=; b=iwVcFyRY8CABG1taFQUXs3T2BEYZtXKBTUAcQKGGSuoyNDFsxlzus8Phx949cEFeI+ TR0StdDECKWGPc7VCLbrBq2hRaP0bcXCxNDnyRk1eDvlaQu2bQ/7ajeRAsP8Z3YRfFht gEhb1hdUyiO1aub8NvKXc60IAnB/fHoVkjygyY3VU/gQq+EwIHiE85KTFiaX58qW8dnT hRg4982tFN91Y8+ol1sO3SxV77ZluCwO74x6W9uHZwN5TMNaTtjw5VE3KO8mX7zgPg9Z w/2sBpuEClBOp4FxAcufnxohre+a0NfcacuC1VmW8n3a/sJJJ27nk/aeeVqiWAW/Yrpx qTeQ== X-Gm-Message-State: AOAM5319OdtziRuMs/FuBHVqrgSm/wIGcCEwdLDp4q63vjONDpKOmtKh Fk3WaK8v51VDXlb8VnAQjbvkI/qPuluzytxOMHmOZE1q3h0VSE4X/0iMkh5Nouq8GDMXi16rsfe +ZzQZMnQauj/+UI8= X-Received: by 2002:a5d:4387:0:b0:1ed:a13a:ef0c with SMTP id i7-20020a5d4387000000b001eda13aef0cmr5798067wrq.62.1645789921677; Fri, 25 Feb 2022 03:52:01 -0800 (PST) X-Google-Smtp-Source: ABdhPJzxdsOBZKB6TtVSCJiDjEoPKDv3L7GJlEpOUn3FGf1Qp9ppyCiHKrBp77TZNcXz9ACVbskxuQ== X-Received: by 2002:a5d:4387:0:b0:1ed:a13a:ef0c with SMTP id i7-20020a5d4387000000b001eda13aef0cmr5798045wrq.62.1645789921416; Fri, 25 Feb 2022 03:52:01 -0800 (PST) Received: from redhat.com ([2.55.145.157]) by smtp.gmail.com with ESMTPSA id m10-20020adfe94a000000b001ef57f562ccsm2018112wrn.51.2022.02.25.03.51.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Feb 2022 03:52:00 -0800 (PST) Date: Fri, 25 Feb 2022 06:51:55 -0500 From: "Michael S. Tsirkin" To: Ard Biesheuvel Subject: Re: [PATCH v3 2/2] virt: vmgenid: introduce driver for reinitializing RNG on VM fork Message-ID: <20220225064445-mutt-send-email-mst@kernel.org> References: <20220224133906.751587-1-Jason@zx2c4.com> <20220224133906.751587-3-Jason@zx2c4.com> MIME-Version: 1.0 In-Reply-To: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=mst@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=170.10.129.124; envelope-from=mst@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.082, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "Jason A. Donenfeld" , kvm@vger.kernel.org, raduweis@amazon.com, linux-hyperv@vger.kernel.org, Dominik Brodowski , KY Srinivasan , Wei Liu , Stephen Hemminger , ben@skyportsystems.com, Dexuan Cui , Eric Biggers , Laszlo Ersek , Eduardo Habkost , adrian@parity.io, Jann Horn , Haiyang Zhang , QEMU Developers , Alexander Graf , "Theodore Y. Ts'o" , colmmacc@amazon.com, berrange@redhat.com, Greg Kroah-Hartman , Linux Kernel Mailing List , Linux Crypto Mailing List , Igor Mammedov , dwmw@amazon.co.uk Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Fri, Feb 25, 2022 at 12:24:05PM +0100, Ard Biesheuvel wrote: > On Thu, 24 Feb 2022 at 14:39, Jason A. Donenfeld wrote: > > > > VM Generation ID is a feature from Microsoft, described at > > , and supported by > > Hyper-V and QEMU. Its usage is described in Microsoft's RNG whitepaper, > > , 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 . > > And there are hooks for this in libvirt as well, described in > > . > > > > Note, however, that the treatment of this as a UUID is considered to be > > an accidental QEMU nuance, per > > , > > 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 > > Cc: Laszlo Ersek > > Cc: Daniel P. Berrangé > > Cc: Dominik Brodowski > > Cc: Ard Biesheuvel > > Signed-off-by: Jason A. Donenfeld > > --- > > 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 . All Rights Reserved. > > + * Copyright (C) 2020 Amazon. All rights reserved. > > + * Copyright (C) 2018 Red Hat Inc. All rights reserved. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +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 > >