Re: [RFC PATCH 0/1] Implement entropy leak reporting for virtio-rng

[bchalios]

On 4/3/23 4:16 PM, "Jason A. Donenfeld"  wrote:

CAUTION: This email originated from outside of the organization. Do not click 
links or open attachments unless you can confirm the sender and know the 
content is safe.



On Mon, Apr 3, 2023 at 4:15 PM Jason A. Donenfeld  wrote:
>
> Hi Babis,
>
> Why are you resending this? As I mentioned before, I'm going to move
> forward in implementing this feature in a way that actually works with
> the RNG. I'll use your RFC patch as a base, but I think beyond that, I
> can take it from here.

Grrr, sorry! This is for QEMU! I understand.

(Kernel ends from me are forthcoming.)

Jason



Hey Jason,

Good to hear from you. Yeap, I thought it would be nice to be able to test this 
using QEMU (apart from Firecracker).

Cheers,
Babis 

Re: [PATCH 0/2] vmgenid: add generation counter

[bchalios]

On 8/3/22 5:36 PM, "Michael S. Tsirkin"  wrote:

CAUTION: This email originated from outside of the organization. Do not click 
links or open attachments unless you can confirm the sender and know the 
content is safe.



On Wed, Aug 03, 2022 at 03:41:45PM +0200, bchal...@amazon.es wrote:
> From: Babis Chalios 
>
> VM generation ID exposes a GUID inside the VM which changes every time a
> VM restore is happening. Typically, this GUID is used by the guest
> kernel to re-seed its internal PRNG. As a result, this value cannot be
> exposed in guest user-space as a notification mechanism for VM restore
> events.
>
> This patch set extends vmgenid to introduce a 32 bits generation counter
> whose purpose is to be used as a VM restore notification mechanism for
> the guest user-space.
>
> It is true that such a counter could be implemented entirely by the
> guest kernel, but this would rely on the vmgenid ACPI notification to
> trigger the counter update, which is inherently racy. Exposing this
> through the monitor allows the updated value to be in-place before
> resuming the vcpus, so interested user-space code can (atomically)
> observe the update without relying on the ACPI notification.

Producing another 4 bytes is not really the issue, the issue
is how does guest consume this.
So I would like this discussion to happen on the linux kernel mailing
list not just here.  Can you post the linux patch please?



CCed you in the Linux patch thread.





> Babis Chalios (2):
>vmgenid: make device data size configurable
>vmgenid: add generation counter
>
>   docs/specs/vmgenid.txt| 101 ++
>   hw/acpi/vmgenid.c | 145 +++---
>   include/hw/acpi/vmgenid.h |  23 --
>   3 files changed, 204 insertions(+), 65 deletions(-)
>
> --
> 2.37.1
>
> Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 
5, 28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja 
M-401234 . CIF B84570936



Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 
28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja 
M-401234 . CIF B84570936

[PATCH 1/2] vmgenid: make device data size configurable

[bchalios]

From: Babis Chalios 

When allocating memory for the device data the assumption is we are
dealing with 4K pages. Make this configurable, so that other
architectures can be handled.

Note, than in the original spec this is not a requirement, however, it
is useful for implementing the generation counter (see next commit in
this patchset) in architectures with page sizes other than 4K.

Signed-off-by: Babis Chalios 
---
 docs/specs/vmgenid.txt|  8 +++---
 hw/acpi/vmgenid.c | 57 ---
 include/hw/acpi/vmgenid.h | 14 +-
 3 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
index 80ff69f31c..5274b4c895 100644
--- a/docs/specs/vmgenid.txt
+++ b/docs/specs/vmgenid.txt
@@ -225,14 +225,16 @@ following diagram:
 Device Usage:
 -
 
-The device has one property, which may be only be set using the command line:
+The device has two properties, which may be only be set using the command line:
 
-  guid - sets the value of the GUID.  A special value "auto" instructs
+  guid - sets the value of the GUID. A special value "auto" instructs
  QEMU to generate a new random GUID.
+  page-size - sets the target machines page size. Currently accepted values
+  are 4096 (default) and 65536.
 
 For example:
 
-  QEMU  -device vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87"
+  QEMU  -device 
vmgenid,guid="324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87",page-size=65536
   QEMU  -device vmgenid,guid=auto
 
 The property may be queried via QMP/HMP:
diff --git a/hw/acpi/vmgenid.c b/hw/acpi/vmgenid.c
index 0c9f158ac9..ac2b116b6a 100644
--- a/hw/acpi/vmgenid.c
+++ b/hw/acpi/vmgenid.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/visitor.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-machine.h"
 #include "qemu/module.h"
@@ -35,7 +36,7 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray 
*table_data, GArray *guid,
 /* Fill in the GUID values.  These need to be converted to little-endian
  * first, since that's what the guest expects
  */
-g_array_set_size(guid, VMGENID_FW_CFG_SIZE - ARRAY_SIZE(guid_le.data));
+g_array_set_size(guid, vms->page_size - ARRAY_SIZE(guid_le.data));
 guid_le = qemu_uuid_bswap(vms->guid);
 /* The GUID is written at a fixed offset into the fw_cfg file
  * in order to implement the "OVMF SDT Header probe suppressor"
@@ -94,7 +95,8 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray 
*table_data, GArray *guid,
 g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
 
 /* Allocate guest memory for the Data fw_cfg blob */
-bios_linker_loader_alloc(linker, VMGENID_GUID_FW_CFG_FILE, guid, 4096,
+bios_linker_loader_alloc(linker, VMGENID_GUID_FW_CFG_FILE, guid,
+ vms->page_size,
  false /* page boundary, high memory */);
 
 /* Patch address of GUID fw_cfg blob into the ADDR fw_cfg blob
@@ -124,8 +126,7 @@ void vmgenid_build_acpi(VmGenIdState *vms, GArray 
*table_data, GArray *guid,
 void vmgenid_add_fw_cfg(VmGenIdState *vms, FWCfgState *s, GArray *guid)
 {
 /* Create a read-only fw_cfg file for GUID */
-fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data,
-VMGENID_FW_CFG_SIZE);
+fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE, guid->data, vms->page_size);
 /* Create a read-write fw_cfg file for Address */
 fw_cfg_add_file_callback(s, VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, NULL,
  vms->vmgenid_addr_le,
@@ -215,8 +216,56 @@ static void vmgenid_realize(DeviceState *dev, Error **errp)
 vmgenid_update_guest(vms);
 }
 
+static void get_page_size(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+Property *prop = opaque;
+uint32_t *page_size = object_field_prop_ptr(obj, prop);
+
+visit_type_uint32(v, name, page_size, errp);
+}
+
+static void set_page_size(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+Property *prop = opaque;
+uint32_t *page_size = object_field_prop_ptr(obj, prop);
+uint32_t val;
+char str[10];
+
+if (!visit_type_uint32(v, name, , errp)) {
+return;
+}
+
+switch (val) {
+case 4096:
+case 65536:
+*page_size = val;
+break;
+default:
+snprintf(str, 10, "%d", val);
+error_set_from_qdev_prop_error(errp, EINVAL, obj, name, str);
+}
+}
+
+static void set_default_page_size(ObjectProperty *op, const Property *prop)
+{
+object_property_set_default_uint(op, VMGENID_DEFAULT_FW_PAGE_SIZE);
+}
+
+const PropertyInfo vmgenid_prop_page_size = {
+.name = "uint32",
+.description = "Page size to use for allocating device memory. \""
+   "\"Valid values: 4096(default) 65536",
+.get = get_page_size,
+.set = set_page_size,
+.set_default_value = 

[PATCH 2/2] vmgenid: add generation counter

[bchalios]

From: Babis Chalios 

Some user-space applications such as user-space PRNGs or applications
that keep world-unique data need to be notified about VM restore events,
so that they modify their internal state. However, the vmgenid 128-bits
UUID is consumed by the guest kernel to reseed its RNG, hence it is not
suitable to be exposed to user-space.

In order to address the user-space needs this commit extends the vmgenid
to include a 32-bits generation counter. The value of the counter is
meant to be increased every time we start a new VM from a saved state.

It is stored in a page aligned, one page-long buffer, so that it allows
the guest kernel to expose it as an mmap-able device to the user-space
for low latency access, out of the notification path.

Signed-off-by: Babis Chalios 
---
 docs/specs/vmgenid.txt| 97 +++
 hw/acpi/vmgenid.c | 96 +++---
 include/hw/acpi/vmgenid.h |  9 +++-
 3 files changed, 144 insertions(+), 58 deletions(-)

diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt
index 5274b4c895..3cf6d0e191 100644
--- a/docs/specs/vmgenid.txt
+++ b/docs/specs/vmgenid.txt
@@ -20,6 +20,15 @@ guest operating system notices the change, and is then able 
to react as
 appropriate by marking its copies of distributed databases as dirty,
 re-initializing its random number generator etc.
 
+Currently, in Linux the UUID exposed by vmgenid is consumed by the guest kernel
+to reseed its RNG and not exposed in the user-space, hence leaving applications
+without a mechanism which would help them detect a VM restore event.
+
+To address the use-case of user-space applications we extend the vmgenid
+specification to include a 32-bit generation counter. The counter needs to be
+incremented every time a VM is loaded from a saved state by the user. The guest
+kernel needs to expose to user-space applications which should monitor it for
+detecting the VM-load event.
 
 Requirements
 
@@ -53,12 +62,24 @@ R2 to R5. [These AML requirements are isolated well enough 
in the Microsoft
 R6. The hypervisor shall expose a _HID (hardware identifier) object in the
 VMGenId device's scope that is unique to the hypervisor vendor.
 
+The generation counter and the buffer holding it shall adhere to the same
+requirements as the generation ID. Moreover,
+
+R7. The BIOS will need to include an object below the device named "CTRA",
+which, similarly to "ADDR", will evaluate to a package of two integers. The
+first integer must be the low 32-bits of the guest physical address of the
+buffer that contains generation counter. The second integer must be the
+hight 32 bits of the guest physical address of the same buffer.
+
+R8. The buffer holding the generation counter will be page aligned and 1 page
+long.
+
 
 QEMU Implementation
 ---
 
 The above-mentioned specification does not dictate which ACPI descriptor table
-will contain the VM Generation ID device.  Other implementations (Hyper-V and
+will contain the VM Generation ID device. Other implementations (Hyper-V and
 Xen) put it in the main descriptor table (Differentiated System Description
 Table or DSDT).  For ease of debugging and implementation, we have decided to
 put it in its own Secondary System Description Table, or SSDT.
@@ -72,8 +93,8 @@ ASL+ Optimizing Compiler version 20150717-64
 Copyright (c) 2000 - 2015 Intel Corporation
 
 Reading ACPI table from file /sys/firmware/acpi/tables/SSDT - Length
-0198 (0xC6)
-ACPI: SSDT 0x C6 (v01 BOCHS  VMGENID  0001 BXPC
+0236 (0xEC)
+ACPI: SSDT 0x EC (v01 BOCHS  VMGENID  0001 BXPC
 0001)
 Acpi table [SSDT] successfully installed and loaded
 Pass 1 parse of [SSDT]
@@ -95,9 +116,9 @@ ASL Output:./SSDT.dsl - 1631 bytes
  *
  * Original Table Header:
  * Signature"SSDT"
- * Length   0x00CA (202)
+ * Length   0x00EC (236)
  * Revision 0x01
- * Checksum 0x4B
+ * Checksum 0x63
  * OEM ID   "BOCHS "
  * OEM Table ID "VMGENID"
  * OEM Revision 0x0001 (1)
@@ -133,6 +154,14 @@ DefinitionBlock ("/sys/firmware/acpi/tables/SSDT.aml", 
"SSDT", 1, "BOCHS ",
 Index (Local0, One) = Zero
 Return (Local0)
 }
+
+Method (CTRA, 0, NotSerialized)
+{
+Local0 = Package (0x02) {}
+Local0 [Zero] = (VGIA + 0x1000)
+Local0 [One] = Zero
+Return (Local0)
+}
 }
 }
 
@@ -161,7 +190,8 @@ More information about fw_cfg can be found in 
"docs/specs/fw_cfg.txt"
 
 Two fw_cfg blobs are used in this case:
 
-/etc/vmgenid_guid - contains the actual VM Generation ID GUID
+/etc/vmgenid_data - contains the actual VM Generation ID GUID and Generation
+counter
   - read-only 

[PATCH 0/2] vmgenid: add generation counter

[bchalios]

From: Babis Chalios 

VM generation ID exposes a GUID inside the VM which changes every time a
VM restore is happening. Typically, this GUID is used by the guest
kernel to re-seed its internal PRNG. As a result, this value cannot be
exposed in guest user-space as a notification mechanism for VM restore
events.

This patch set extends vmgenid to introduce a 32 bits generation counter
whose purpose is to be used as a VM restore notification mechanism for
the guest user-space.

It is true that such a counter could be implemented entirely by the
guest kernel, but this would rely on the vmgenid ACPI notification to
trigger the counter update, which is inherently racy. Exposing this
through the monitor allows the updated value to be in-place before
resuming the vcpus, so interested user-space code can (atomically)
observe the update without relying on the ACPI notification.

Babis Chalios (2):
  vmgenid: make device data size configurable
  vmgenid: add generation counter

 docs/specs/vmgenid.txt| 101 ++
 hw/acpi/vmgenid.c | 145 +++---
 include/hw/acpi/vmgenid.h |  23 --
 3 files changed, 204 insertions(+), 65 deletions(-)

-- 
2.37.1

Amazon Spain Services sociedad limitada unipersonal, Calle Ramirez de Prado 5, 
28045 Madrid. Registro Mercantil de Madrid . Tomo 22458 . Folio 102 . Hoja 
M-401234 . CIF B84570936