On 16.11.20 16:34, Catangiu, Adrian Costin wrote:
- Background
The VM Generation ID is a feature defined by Microsoft (paper:
http://go.microsoft.com/fwlink/?LinkId=260709) and supported by
multiple hypervisor vendors.
The feature is required in virtualized environments by apps that work
with local copies/caches of world-unique data such as random values,
uuids, monotonically increasing counters, etc.
Such apps can be negatively affected by VM snapshotting when the VM
is either cloned or returned to an earlier point in time.
The VM Generation ID is a simple concept meant to alleviate the issue
by providing a unique ID that changes each time the VM is restored
from a snapshot. The hw provided UUID value can be used to
differentiate between VMs or different generations of the same VM.
- Problem
The VM Generation ID is exposed through an ACPI device by multiple
hypervisor vendors but neither the vendors or upstream Linux have no
default driver for it leaving users to fend for themselves.
Furthermore, simply finding out about a VM generation change is only
the starting point of a process to renew internal states of possibly
multiple applications across the system. This process could benefit
from a driver that provides an interface through which orchestration
can be easily done.
- Solution
This patch is a driver that exposes a monotonic incremental Virtual
Machine Generation u32 counter via a char-dev FS interface that
provides sync and async VmGen counter updates notifications. It also
provides VmGen counter retrieval and confirmation mechanisms.
The hw provided UUID is not exposed to userspace, it is internally
used by the driver to keep accounting for the exposed VmGen counter.
The counter starts from zero when the driver is initialized and
monotonically increments every time the hw UUID changes (the VM
generation changes).
On each hw UUID change, the new hypervisor-provided UUID is also fed
to the kernel RNG.
This patch builds on top of Or Idgar <orid...@gmail.com>'s proposal
https://lkml.org/lkml/2018/3/1/498
- Future improvements
Ideally we would want the driver to register itself based on devices'
_CID and not _HID, but unfortunately I couldn't find a way to do that.
The problem is that ACPI device matching is done by
'__acpi_match_device()' which exclusively looks at
'acpi_hardware_id *hwid'.
There is a path for platform devices to match on _CID when _HID is
'PRP0001' - but this is not the case for the Qemu vmgenid device.
Guidance and help here would be greatly appreciated.
That one is pretty important IMHO. How about the following (probably
pretty mangled) patch? That seems to work for me. The ACPI change would
obviously need to be its own stand alone change and needs proper
assessment whether it could possibly break any existing systems.
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 1682f8b454a2..452443d79d87 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -748,7 +748,7 @@ static bool __acpi_match_device(struct acpi_device
*device,
/* First, check the ACPI/PNP IDs provided by the caller. */
if (acpi_ids) {
for (id = acpi_ids; id->id[0] || id->cls; id++) {
- if (id->id[0] && !strcmp((char *)id->id,
hwid->id))
+ if (id->id[0] && !strncmp((char *)id->id,
hwid->id, ACPI_ID_LEN - 1))
goto out_acpi_match;
if (id->cls && __acpi_match_device_cls(id,
hwid))
goto out_acpi_match;
diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index 75a787da8aad..0bfa422cf094 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -356,7 +356,8 @@ static void vmgenid_acpi_notify(struct acpi_device
*device, u32 event)
}
static const struct acpi_device_id vmgenid_ids[] = {
- {"QEMUVGID", 0},
+ /* This really is VM_Gen_Counter, but we can only match 8 characters */
+ {"VM_GEN_C", 0},
{"", 0},
};
- v1 -> v2:
Please put the change log below your Signed-off-by line and separate it
with a "---" line. That way, git am will ignore the change log on apply.
- expose to userspace a monotonically increasing u32 Vm Gen Counter
instead of the hw VmGen UUID
- since the hw/hypervisor-provided 128-bit UUID is not public
anymore, add it to the kernel RNG as device randomness
- insert driver page containing Vm Gen Counter in the user vma in
the driver's mmap handler instead of using a fault handler
- turn driver into a misc device driver to auto-create /dev/vmgenid
- change ioctl arg to avoid leaking kernel structs to userspace
- update documentation
- various nits (license, unnecessary casting, Kconfig, others)
- rebase on top of linus latest
Signed-off-by: Adrian Catangiu <aca...@amazon.com>
---
Documentation/virt/vmgenid.rst | 228 ++++++++++++++++++++++++
drivers/virt/Kconfig | 17 ++
drivers/virt/Makefile | 1 +
drivers/virt/vmgenid.c | 390 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vmgenid.h | 13 ++
5 files changed, 649 insertions(+)
create mode 100644 Documentation/virt/vmgenid.rst
create mode 100644 drivers/virt/vmgenid.c
create mode 100644 include/uapi/linux/vmgenid.h
diff --git a/Documentation/virt/vmgenid.rst b/Documentation/virt/vmgenid.rst
new file mode 100644
index 0000000..603e8a5
--- /dev/null
+++ b/Documentation/virt/vmgenid.rst
@@ -0,0 +1,228 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+VMGENID
+============
+
+The VM Generation ID is a feature defined by Microsoft (paper:
+http://go.microsoft.com/fwlink/?LinkId=260709) and supported by
+multiple hypervisor vendors.
+
+The feature is required in virtualized environments by apps that work
+with local copies/caches of world-unique data such as random values,
+uuids, monotonically increasing counters, etc.
+Such apps can be negatively affected by VM snapshotting when the VM
+is either cloned or returned to an earlier point in time.
+
+The VM Generation ID is a simple concept meant to alleviate the issue
+by providing a unique ID that changes each time the VM is restored
+from a snapshot. The hw provided UUID value can be used to
+differentiate between VMs or different generations of the same VM.
+
+The VM Generation ID is exposed through an ACPI device by multiple
+hypervisor vendors. The driver for it lives at
+``drivers/virt/vmgenid.c``
+
+The driver exposes a monotonic incremental Virtual Machine Generation
+u32 counter via a char-dev FS interface that provides sync and async
+VmGen counter updates notifications. It also provides VmGen counter
+retrieval and confirmation mechanisms.
+
+The hw provided UUID is not exposed to userspace, it is internally
+used by the driver to keep accounting for the exposed VmGen counter.
+The counter starts from zero when the driver is initialized and
+monotonically increments every time the hw UUID changes (the VM
+generation changes).
+
+On each hw UUID change, the new UUID is also fed to the kernel RNG.
+
+Driver interface:
+
+``open()``:
+ When the device is opened, a copy of the current Vm-Gen-Id (counter)
+ is associated with the open file descriptor. The driver now tracks
+ this file as an independent *watcher*. The driver tracks how many
+ watchers are aware of the latest Vm-Gen-Id counter and how many of
+ them are *outdated*; outdated being those that have lived through
+ a Vm-Gen-Id change but not yet confirmed the new generation counter.
+
+``read()``:
+ Read is meant to provide the *new* VM generation counter when a
+ generation change takes place. The read operation blocks until the
+ associated counter is no longer up to date - until HW vm gen id
+ changes - at which point the new counter is provided/returned.
+ Nonblocking ``read()`` uses ``EAGAIN`` to signal that there is no
+ *new* counter value available. The generation counter is considered
+ *new* for each open file descriptor that hasn't confirmed the new
+ value, following a generation change. Therefore, once a generation
+ change takes place, all ``read()`` calls will immediately return the
+ new generation counter and will continue to do so until the
+ new value is confirmed back to the driver through ``write()``.
+ Partial reads are not allowed - read buffer needs to be at least
+ ``sizeof(unsigned)`` in size.
+
+``write()``:
+ Write is used to confirm the up-to-date Vm Gen counter back to the
+ driver.
+ Following a VM generation change, all existing watchers are marked
+ as *outdated*. Each file descriptor will maintain the *outdated*
+ status until a ``write()`` confirms the up-to-date counter back to
+ the driver.
+ Partial writes are not allowed - write buffer should be exactly
+ ``sizeof(unsigned)`` in size.
+
+``poll()``:
+ Poll is implemented to allow polling for generation counter updates.
+ Such updates result in ``EPOLLIN`` polling status until the new
+ up-to-date counter is confirmed back to the driver through a
+ ``write()``.
+
+``ioctl()``:
+ The driver also adds support for tracking count of open file
+ descriptors that haven't acknowledged a generation counter update.
+ This is exposed through two IOCTLs:
+
+ - VMGENID_GET_OUTDATED_WATCHERS: immediately returns the number of
+ *outdated* watchers - number of file descriptors that were open
+ during a VM generation change, and which have not yet confirmed the
+ new generation counter.
+ - VMGENID_WAIT_WATCHERS: blocks until there are no more *outdated*
+ watchers, or if a ``timeout`` argument is provided, until the
+ timeout expires.
+
+``mmap()``:
+ The driver supports ``PROT_READ, MAP_SHARED`` mmaps of a single page
+ in size. The first 4 bytes of the mapped page will contain an
+ up-to-date copy of the VM generation counter.
+ The mapped memory can be used as a low-latency generation counter
+ probe mechanism in critical sections - see examples.
+
+``close()``:
+ Removes the file descriptor as a Vm generation counter watcher.
+
+Example application workflows
+-----------------------------
+
+1) Watchdog thread simplified example::
+
+ void watchdog_thread_handler(int *thread_active)
+ {
+ unsigned genid;
+ int fd = open("/dev/vmgenid", O_RDWR | O_CLOEXEC, S_IRUSR |
S_IWUSR);
+
+ do {
+ // read new gen ID - blocks until VM generation changes
+ read(fd, &genid, sizeof(genid));
+
+ // because of VM generation change, we need to rebuild
world
+ reseed_app_env();
+
+ // confirm we're done handling gen ID update
+ write(fd, &genid, sizeof(genid));
+ } while (atomic_read(thread_active));
+
+ close(fd);
+ }
+
+2) ASYNC simplified example::
+
+ void handle_io_on_vmgenfd(int vmgenfd)
+ {
+ unsigned genid;
+
+ // because of VM generation change, we need to rebuild world
+ reseed_app_env();
+
+ // read new gen ID - we need it to confirm we've handled update
+ read(fd, &genid, sizeof(genid));
This is racy in case two consecutive snapshots happen. The read needs to
go before the reseed.
+
+ // confirm we're done handling the gen ID update
+ write(fd, &genid, sizeof(genid));
+ }
+
+ int main() {
+ int epfd, vmgenfd;
+ struct epoll_event ev;
+
+ epfd = epoll_create(EPOLL_QUEUE_LEN);
+
+ vmgenfd = open("/dev/vmgenid",
+ O_RDWR | O_CLOEXEC | O_NONBLOCK,
+ S_IRUSR | S_IWUSR);
+
+ // register vmgenid for polling
+ ev.events = EPOLLIN;
+ ev.data.fd = vmgenfd;
+ epoll_ctl(epfd, EPOLL_CTL_ADD, vmgenfd, &ev);
+
+ // register other parts of your app for polling
+ // ...
+
+ while (1) {
+ // wait for something to do...
+ int nfds = epoll_wait(epfd, events,
+ MAX_EPOLL_EVENTS_PER_RUN,
+ EPOLL_RUN_TIMEOUT);
+ if (nfds < 0) die("Error in epoll_wait!");
+
+ // for each ready fd
+ for(int i = 0; i < nfds; i++) {
+ int fd = events[i].data.fd;
+
+ if (fd == vmgenfd)
+ handle_io_on_vmgenfd(vmgenfd);
+ else
+ handle_some_other_part_of_the_app(fd);
+ }
+ }
+
+ return 0;
+ }
+
+3) Mapped memory polling simplified example::
+
+ /*
+ * app/library function that provides cached secrets
+ */
+ char * safe_cached_secret(app_data_t *app)
+ {
+ char *secret;
+ volatile unsigned *const genid_ptr = get_vmgenid_mapping(app);
+ again:
+ secret = __cached_secret(app);
+
+ if (unlikely(*genid_ptr != app->cached_genid)) {
+ // rebuild world then confirm the genid update (thru
write)
+ rebuild_caches(app);
+
+ app->cached_genid = *genid_ptr;
This is racy again. You need to read the genid before rebuild and set it
here.
+ ack_vmgenid_update(app);
+
+ goto again;
+ }
+
+ return secret;
+ }
+
+4) Orchestrator simplified example::
+
+ /*
+ * orchestrator - manages multiple apps and libraries used by a service
+ * and tries to make sure all sensitive components gracefully handle
+ * VM generation changes.
+ * Following function is called on detection of a VM generation change.
+ */
+ int handle_vmgen_update(int vmgen_fd, unsigned new_gen_id)
+ {
+ // pause until all components have handled event
+ pause_service();
+
+ // confirm *this* watcher as up-to-date
+ write(vmgen_fd, &new_gen_id, sizeof(unsigned));
+
+ // wait for all *others* for at most 5 seconds.
+ ioctl(vmgen_fd, VMGENID_WAIT_WATCHERS, 5000);
+
+ // all apps on the system have rebuilt worlds
+ resume_service();
+ }
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 80c5f9c1..5d5f37b 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -13,6 +13,23 @@ menuconfig VIRT_DRIVERS
if VIRT_DRIVERS
+config VMGENID
+ tristate "Virtual Machine Generation ID driver"
+ depends on ACPI
+ default N
+ help
+ This is a Virtual Machine Generation ID driver which provides
+ a virtual machine generation counter. The driver exposes FS ops
+ on /dev/vmgenid through which it can provide information and
+ notifications on VM generation changes that happen on snapshots
+ or cloning.
+ This enables applications and libraries that store or cache
+ sensitive information, to know that they need to regenerate it
+ after process memory has been exposed to potential copying.
+
+ To compile this driver as a module, choose M here: the
+ module will be called vmgenid.
+
config FSL_HV_MANAGER
tristate "Freescale hypervisor management driver"
depends on FSL_SOC
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index f28425c..889be01 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 0000000..75a787d
--- /dev/null
+++ b/drivers/virt/vmgenid.c
@@ -0,0 +1,390 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtual Machine Generation ID driver
+ *
+ * Copyright (C) 2018 Red Hat Inc. All rights reserved.
+ *
+ * Copyright (C) 2020 Amazon. All rights reserved.
+ *
+ * Authors:
+ * Adrian Catangiu <aca...@amazon.com>
+ * Or Idgar <orid...@gmail.com>
+ * Gal Hammer <gham...@redhat.com>
+ *
+ */
+#include <linux/acpi.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/poll.h>
+#include <linux/random.h>
+#include <linux/uuid.h>
+#include <linux/vmgenid.h>
+
+#define DEV_NAME "vmgenid"
+ACPI_MODULE_NAME(DEV_NAME);
+
+struct dev_data {
+ struct miscdevice misc_dev;
+ unsigned long map_buf;
+ wait_queue_head_t read_wait;
+ unsigned int generation_counter;
+
+ void *uuid_iomap;
+ uuid_t uuid;
+
+ atomic_t watchers;
+ atomic_t outdated_watchers;
+ wait_queue_head_t outdated_wait;
+};
+
+struct file_data {
+ struct dev_data *dev_data;
+ unsigned int acked_gen_counter;
+};
+
+static void vmgenid_put_outdated_watchers(struct dev_data *priv)
+{
+ if (atomic_dec_and_test(&priv->outdated_watchers))
+ wake_up_interruptible(&priv->outdated_wait);
+}
+
+static int vmgenid_open(struct inode *inode, struct file *file)
+{
+ struct dev_data *priv =
+ container_of(file->private_data, struct dev_data, misc_dev);
+ struct file_data *file_data =
+ kzalloc(sizeof(struct file_data), GFP_KERNEL);
+
+ if (!file_data)
+ return -ENOMEM;
+
+ file_data->acked_gen_counter = priv->generation_counter;
+ file_data->dev_data = priv;
+
+ file->private_data = file_data;
+ atomic_inc(&priv->watchers);
+
+ return 0;
+}
+
+static int vmgenid_close(struct inode *inode, struct file *file)
+{
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+
+ if (file_data->acked_gen_counter != priv->generation_counter)
+ vmgenid_put_outdated_watchers(priv);
Is this racy? Could there be a snapshot notification coming between the
branch and the put?
+ atomic_dec(&priv->watchers);
+ kfree(file_data);
+
+ return 0;
+}
+
+static ssize_t
+vmgenid_read(struct file *file, char __user *ubuf, size_t nbytes, loff_t *ppos)
+{
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+ ssize_t ret;
+
+ if (nbytes == 0)
+ return 0;
+ /* disallow partial reads */
+ if (nbytes < sizeof(priv->generation_counter))
+ return -EINVAL;
+
+ if (file_data->acked_gen_counter == priv->generation_counter) {
+ if (file->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+ ret = wait_event_interruptible(
+ priv->read_wait,
+ file_data->acked_gen_counter != priv->generation_counter
+ );
+ if (ret)
+ return ret;
+ }
+
+ nbytes = sizeof(priv->generation_counter);
+ ret = copy_to_user(ubuf, &priv->generation_counter, nbytes);
+ if (ret)
+ return -EFAULT;
+
+ return nbytes;
+}
+
+static ssize_t vmgenid_write(struct file *file, const char __user *ubuf,
+ size_t count, loff_t *ppos)
+{
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+ unsigned int acked_gen_count;
+
+ /* disallow partial writes */
+ if (count != sizeof(acked_gen_count))
+ return -EINVAL;
+ if (copy_from_user(&acked_gen_count, ubuf, count))
+ return -EFAULT;
+ /* wrong gen-counter acknowledged */
+ if (acked_gen_count != priv->generation_counter)
+ return -EINVAL;
+
+ if (file_data->acked_gen_counter != priv->generation_counter) {
+ /* update local view of UUID */
+ file_data->acked_gen_counter = acked_gen_count;
+ vmgenid_put_outdated_watchers(priv);
Same question here: What if there is a notification between the branch
and the put?
+ }
+
+ return (ssize_t)count;
+}
+
+static __poll_t
+vmgenid_poll(struct file *file, poll_table *wait)
+{
+ __poll_t mask = 0;
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+
+ if (file_data->acked_gen_counter != priv->generation_counter)
+ return EPOLLIN | EPOLLRDNORM;
+
+ poll_wait(file, &priv->read_wait, wait);
+
+ if (file_data->acked_gen_counter != priv->generation_counter)
+ mask = EPOLLIN | EPOLLRDNORM;
+
+ return mask;
+}
+
+static long vmgenid_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+ unsigned long timeout_ns = arg * NSEC_PER_MSEC;
+ ktime_t until = ktime_set(0, timeout_ns);
+ int ret;
+
+ switch (cmd) {
+ case VMGENID_GET_OUTDATED_WATCHERS:
+ ret = atomic_read(&priv->outdated_watchers);
+ break;
+ case VMGENID_WAIT_WATCHERS:
+ ret = wait_event_interruptible_hrtimeout(
+ priv->outdated_wait,
+ !atomic_read(&priv->outdated_watchers),
+ until
+ );
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int vmgenid_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct file_data *file_data = file->private_data;
+ struct dev_data *priv = file_data->dev_data;
+
+ if (vma->vm_pgoff != 0 || vma_pages(vma) > 1)
+ return -EINVAL;
+
+ if ((vma->vm_flags & VM_WRITE) != 0)
+ return -EPERM;
+
+ vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_flags &= ~VM_MAYWRITE;
+ vma->vm_private_data = file_data;
+
+ return vm_insert_page(vma, vma->vm_start,
+ virt_to_page(priv->map_buf));
Is this weird white space introduced by my mail client or your patch? :)
Alex
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
