[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v7 1/7] vhost: Build temporary section list and dere
From: |
Dr. David Alan Gilbert (git) |
Subject: |
[Qemu-devel] [PATCH v7 1/7] vhost: Build temporary section list and deref after commit |
Date: |
Fri, 19 Jan 2018 10:39:18 +0000 |
From: "Dr. David Alan Gilbert" <address@hidden>
Igor spotted that there's a race, where a region that's unref'd
in a _del callback might be free'd before the set_mem_table call in
the _commit callback, and thus the vhost might end up using free memory.
Fix this by building a complete temporary sections list, ref'ing every
section (during add and nop) and then unref'ing the whole list right
at the end of commit.
Signed-off-by: Dr. David Alan Gilbert <address@hidden>
Reviewed-by: Igor Mammedov <address@hidden>
---
hw/virtio/vhost.c | 73 ++++++++++++++++++++++++++++++-----------------
include/hw/virtio/vhost.h | 2 ++
2 files changed, 49 insertions(+), 26 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index e4290ce93d..610bfe6945 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -621,6 +621,8 @@ static void vhost_begin(MemoryListener *listener)
memory_listener);
dev->mem_changed_end_addr = 0;
dev->mem_changed_start_addr = -1;
+ dev->tmp_sections = NULL;
+ dev->n_tmp_sections = 0;
}
static void vhost_commit(MemoryListener *listener)
@@ -629,17 +631,25 @@ static void vhost_commit(MemoryListener *listener)
memory_listener);
hwaddr start_addr = 0;
ram_addr_t size = 0;
+ MemoryRegionSection *old_sections;
+ int n_old_sections;
+
uint64_t log_size;
int r;
+ old_sections = dev->mem_sections;
+ n_old_sections = dev->n_mem_sections;
+ dev->mem_sections = dev->tmp_sections;
+ dev->n_mem_sections = dev->n_tmp_sections;
+
if (!dev->memory_changed) {
- return;
+ goto out;
}
if (!dev->started) {
- return;
+ goto out;
}
if (dev->mem_changed_start_addr > dev->mem_changed_end_addr) {
- return;
+ goto out;
}
if (dev->started) {
@@ -656,7 +666,7 @@ static void vhost_commit(MemoryListener *listener)
VHOST_OPS_DEBUG("vhost_set_mem_table failed");
}
dev->memory_changed = false;
- return;
+ goto out;
}
log_size = vhost_get_log_size(dev);
/* We allocate an extra 4K bytes to log,
@@ -675,6 +685,27 @@ static void vhost_commit(MemoryListener *listener)
vhost_dev_log_resize(dev, log_size);
}
dev->memory_changed = false;
+
+out:
+ /* Deref the old list of sections, this must happen _after_ the
+ * vhost_set_mem_table to ensure the client isn't still using the
+ * section we're about to unref.
+ */
+ while (n_old_sections--) {
+ memory_region_unref(old_sections[n_old_sections].mr);
+ }
+ g_free(old_sections);
+ return;
+}
+
+static void vhost_add_section(struct vhost_dev *dev,
+ MemoryRegionSection *section)
+{
+ ++dev->n_tmp_sections;
+ dev->tmp_sections = g_renew(MemoryRegionSection, dev->tmp_sections,
+ dev->n_tmp_sections);
+ dev->tmp_sections[dev->n_tmp_sections - 1] = *section;
+ memory_region_ref(section->mr);
}
static void vhost_region_add(MemoryListener *listener,
@@ -687,36 +718,31 @@ static void vhost_region_add(MemoryListener *listener,
return;
}
- ++dev->n_mem_sections;
- dev->mem_sections = g_renew(MemoryRegionSection, dev->mem_sections,
- dev->n_mem_sections);
- dev->mem_sections[dev->n_mem_sections - 1] = *section;
- memory_region_ref(section->mr);
+ vhost_add_section(dev, section);
vhost_set_memory(listener, section, true);
}
-static void vhost_region_del(MemoryListener *listener,
+static void vhost_region_nop(MemoryListener *listener,
MemoryRegionSection *section)
{
struct vhost_dev *dev = container_of(listener, struct vhost_dev,
memory_listener);
- int i;
if (!vhost_section(section)) {
return;
}
- vhost_set_memory(listener, section, false);
- memory_region_unref(section->mr);
- for (i = 0; i < dev->n_mem_sections; ++i) {
- if (dev->mem_sections[i].offset_within_address_space
- == section->offset_within_address_space) {
- --dev->n_mem_sections;
- memmove(&dev->mem_sections[i], &dev->mem_sections[i+1],
- (dev->n_mem_sections - i) * sizeof(*dev->mem_sections));
- break;
- }
+ vhost_add_section(dev, section);
+}
+
+static void vhost_region_del(MemoryListener *listener,
+ MemoryRegionSection *section)
+{
+ if (!vhost_section(section)) {
+ return;
}
+
+ vhost_set_memory(listener, section, false);
}
static void vhost_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
@@ -783,11 +809,6 @@ static void vhost_iommu_region_del(MemoryListener
*listener,
}
}
-static void vhost_region_nop(MemoryListener *listener,
- MemoryRegionSection *section)
-{
-}
-
static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
struct vhost_virtqueue *vq,
unsigned idx, bool enable_log)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 467dc7794b..21f3022499 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -54,6 +54,8 @@ struct vhost_dev {
struct vhost_memory *mem;
int n_mem_sections;
MemoryRegionSection *mem_sections;
+ int n_tmp_sections;
+ MemoryRegionSection *tmp_sections;
struct vhost_virtqueue *vqs;
int nvqs;
/* the first virtqueue which would be used by this vhost dev */
--
2.14.3
- [Qemu-devel] [PATCH v7 0/7] Rework vhost memory region updates, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 1/7] vhost: Build temporary section list and deref after commit,
Dr. David Alan Gilbert (git) <=
- [Qemu-devel] [PATCH v7 2/7] vhost: Simplify ring verification checks, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 3/7] vhost: Merge sections added to temporary list, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 4/7] vhost: Regenerate region list from changed sections list, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 5/7] vhost: Clean out old vhost_set_memory and friends, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 7/7] vhost: Move log_dirty check, Dr. David Alan Gilbert (git), 2018/01/19
- [Qemu-devel] [PATCH v7 6/7] vhost: Merge and delete unused callbacks, Dr. David Alan Gilbert (git), 2018/01/19