Update shmem_list to be able to track
active mappings on VIRTIO shared memory
regions. This allows to verify that new
mapping request received from backends
do not overlap. If they do, the request
shall fail in order to adhere to the specs.
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
hw/virtio/vhost-user.c | 31 +++++++++++++-------
hw/virtio/virtio.c | 58 ++++++++++++++++++++++++++++++++++----
include/hw/virtio/virtio.h | 25 ++++++++++++++--
3 files changed, 96 insertions(+), 18 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 338cc942ec..de0bb35257 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -1776,7 +1776,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
int fd)
{
void *addr = 0;
- MemoryRegion *mr = NULL;
+ VirtSharedMemory *shmem = NULL;
if (fd < 0) {
error_report("Bad fd for map");
@@ -1791,22 +1791,29 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
return -EFAULT;
}
- mr = &dev->vdev->shmem_list[vu_mmap->shmid];
+ shmem = &dev->vdev->shmem_list[vu_mmap->shmid];
- if (!mr) {
+ if (!shmem) {
error_report("VIRTIO Shared Memory Region at "
"ID %d unitialized", vu_mmap->shmid);
return -EFAULT;
}
if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
- (vu_mmap->shm_offset + vu_mmap->len) > mr->size) {
+ (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr->size) {
error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64,
vu_mmap->shm_offset, vu_mmap->len);
return -EFAULT;
}
- void *shmem_ptr = memory_region_get_ram_ptr(mr);
+ if (virtio_shmem_map_overlaps(shmem, vu_mmap->shm_offset, vu_mmap->len)) {
+ error_report("Requested memory (%" PRIx64 "+%" PRIx64 ") overalps "
+ "with previously mapped memory",
+ vu_mmap->shm_offset, vu_mmap->len);
+ return -EFAULT;
+ }
+
+ void *shmem_ptr = memory_region_get_ram_ptr(shmem->mr);
addr = mmap(shmem_ptr + vu_mmap->shm_offset, vu_mmap->len,
((vu_mmap->flags & VHOST_USER_FLAG_MAP_R) ? PROT_READ : 0) |
@@ -1818,6 +1825,8 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
return -EFAULT;
}
+ virtio_add_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
+
return 0;
}
@@ -1826,7 +1835,7 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
VhostUserMMap *vu_mmap)
{
void *addr = 0;
- MemoryRegion *mr = NULL;
+ VirtSharedMemory *shmem = NULL;
if (!dev->vdev->shmem_list ||
dev->vdev->n_shmem_regions <= vu_mmap->shmid) {
@@ -1836,22 +1845,22 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
return -EFAULT;
}
- mr = &dev->vdev->shmem_list[vu_mmap->shmid];
+ shmem = &dev->vdev->shmem_list[vu_mmap->shmid];
- if (!mr) {
+ if (!shmem) {
error_report("VIRTIO Shared Memory Region at "
"ID %d unitialized", vu_mmap->shmid);
return -EFAULT;
}
if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
- (vu_mmap->shm_offset + vu_mmap->len) > mr->size) {
+ (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr->size) {
error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64,
vu_mmap->shm_offset, vu_mmap->len);
return -EFAULT;
}
- void *shmem_ptr = memory_region_get_ram_ptr(mr);
+ void *shmem_ptr = memory_region_get_ram_ptr(shmem->mr);
addr = mmap(shmem_ptr + vu_mmap->shm_offset, vu_mmap->len,
PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
@@ -1861,6 +1870,8 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
return -EFAULT;
}
+ virtio_del_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
+
return 0;
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ccc4f2cd75..0e2cd62a15 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3059,15 +3059,52 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
}
-MemoryRegion *virtio_new_shmem_region(VirtIODevice *vdev)
+VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev)
{
- MemoryRegion *mr;
+ VirtSharedMemory *shmem = NULL;
++vdev->n_shmem_regions;
- vdev->shmem_list = g_renew(MemoryRegion, vdev->shmem_list,
+ vdev->shmem_list = g_renew(VirtSharedMemory, vdev->shmem_list,
vdev->n_shmem_regions);
- mr = &vdev->shmem_list[vdev->n_shmem_regions - 1];
- mr = g_new0(MemoryRegion, 1);
- return mr;
+ shmem = &vdev->shmem_list[vdev->n_shmem_regions - 1];
+ shmem = g_new0(VirtSharedMemory, 1);
+ QTAILQ_INIT(&shmem->mapped_regions);
+ return shmem;
+}
+
+void virtio_add_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size)
+{
+ MappedMemoryRegion *mmap = g_new0(MappedMemoryRegion, 1);
+ mmap->offset = offset;
+ mmap->size = int128_make64(size);
+ QTAILQ_REMOVE(&shmem->mapped_regions, mmap, link);
+ g_free(mmap);
+}
+
+void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size)
+{
+ MappedMemoryRegion *mmap = g_new0(MappedMemoryRegion, 1);
+ mmap->offset = offset;
+ mmap->size = int128_make64(size);
+ QTAILQ_INSERT_TAIL(&shmem->mapped_regions, mmap, link);
+ g_free(mmap);
+}
+
+bool virtio_shmem_map_overlaps(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size)
+{
+ MappedMemoryRegion *map_reg;
+ hwaddr new_reg_end = offset + size;
+ QTAILQ_FOREACH(map_reg, &shmem->mapped_regions, link) {
+ hwaddr region_end = map_reg->offset + map_reg->size;
+ if ((map_reg->offset == offset) ||
+ (map_reg->offset < offset && region_end >= offset) ||
+ (offset < map_reg->offset && new_reg_end >= map_reg->offset )) {
+ return true;
+ }
+ }
+ return false;
}
/* A wrapper for use as a VMState .put function */
@@ -4007,11 +4044,20 @@ static void virtio_device_free_virtqueues(VirtIODevice *vdev)
static void virtio_device_instance_finalize(Object *obj)
{
VirtIODevice *vdev = VIRTIO_DEVICE(obj);
+ VirtSharedMemory *shmem = NULL;
+ int i;
virtio_device_free_virtqueues(vdev);
g_free(vdev->config);
g_free(vdev->vector_queues);
+ for (i = 0; i< vdev->n_shmem_regions; i++) {
+ shmem = &vdev->shmem_list[i];
+ while (!QTAILQ_EMPTY(&shmem->mapped_regions)) {
+ MappedMemoryRegion *mmap_reg = QTAILQ_FIRST(&shmem->mapped_regions);
+ QTAILQ_REMOVE(&shmem->mapped_regions, mmap_reg, link);
+ }
+ }
}
static Property virtio_properties[] = {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d4a2f664d9..5b801f33f5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -98,6 +98,21 @@ enum virtio_device_endian {
VIRTIO_DEVICE_ENDIAN_BIG,
};
+struct MappedMemoryRegion {
+ Int128 size;
+ hwaddr offset;
+ QTAILQ_ENTRY(MappedMemoryRegion) link;
+};
+
+typedef struct MappedMemoryRegion MappedMemoryRegion;
+
+struct VirtSharedMemory {
+ MemoryRegion *mr;
+ QTAILQ_HEAD(, MappedMemoryRegion) mapped_regions;
+};
+
+typedef struct VirtSharedMemory VirtSharedMemory;
+
/**
* struct VirtIODevice - common VirtIO structure
* @name: name of the device
@@ -168,7 +183,7 @@ struct VirtIODevice
EventNotifier config_notifier;
bool device_iotlb_enabled;
/* Shared memory region for vhost-user mappings. */
- MemoryRegion *shmem_list;
+ VirtSharedMemory *shmem_list;
int n_shmem_regions;
};
@@ -289,7 +304,13 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
int virtio_save(VirtIODevice *vdev, QEMUFile *f);
-MemoryRegion *virtio_new_shmem_region(VirtIODevice *vdev);
+VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev);
+void virtio_add_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size);
+void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size);
+bool virtio_shmem_map_overlaps(VirtSharedMemory *shmem, hwaddr offset,
+ uint64_t size);
extern const VMStateInfo virtio_vmstate_info;
--
2.45.2On Thu, Sep 12, 2024 at 04:53:32PM +0200, Albert Esteve wrote:
> Update shmem_list to be able to track
> active mappings on VIRTIO shared memory
> regions. This allows to verify that new
> mapping request received from backends
> do not overlap. If they do, the request
> shall fail in order to adhere to the specs.
>
> Signed-off-by: Albert Esteve <aesteve@redhat.com>
> ---
> hw/virtio/vhost-user.c | 31 +++++++++++++-------
> hw/virtio/virtio.c | 58 ++++++++++++++++++++++++++++++++++----
> include/hw/virtio/virtio.h | 25 ++++++++++++++--
> 3 files changed, 96 insertions(+), 18 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 338cc942ec..de0bb35257 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1776,7 +1776,7 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> int fd)
> {
> void *addr = 0;
> - MemoryRegion *mr = NULL;
> + VirtSharedMemory *shmem = NULL;
>
> if (fd < 0) {
> error_report("Bad fd for map");
> @@ -1791,22 +1791,29 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> return -EFAULT;
> }
>
> - mr = &dev->vdev->shmem_list[vu_mmap->shmid];
> + shmem = &dev->vdev->shmem_list[vu_mmap->shmid];
>
> - if (!mr) {
> + if (!shmem) {
> error_report("VIRTIO Shared Memory Region at "
> "ID %d unitialized", vu_mmap->shmid);
> return -EFAULT;
> }
>
> if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
> - (vu_mmap->shm_offset + vu_mmap->len) > mr->size) {
> + (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr->size) {
> error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64,
> vu_mmap->shm_offset, vu_mmap->len);
> return -EFAULT;
> }
>
> - void *shmem_ptr = memory_region_get_ram_ptr(mr);
> + if (virtio_shmem_map_overlaps(shmem, vu_mmap->shm_offset, vu_mmap->len)) {
> + error_report("Requested memory (%" PRIx64 "+%" PRIx64 ") overalps "
> + "with previously mapped memory",
> + vu_mmap->shm_offset, vu_mmap->len);
> + return -EFAULT;
> + }
> +
> + void *shmem_ptr = memory_region_get_ram_ptr(shmem->mr);
>
> addr = mmap(shmem_ptr + vu_mmap->shm_offset, vu_mmap->len,
> ((vu_mmap->flags & VHOST_USER_FLAG_MAP_R) ? PROT_READ : 0) |
> @@ -1818,6 +1825,8 @@ vhost_user_backend_handle_shmem_map(struct vhost_dev *dev,
> return -EFAULT;
> }
>
> + virtio_add_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
> +
> return 0;
> }
>
> @@ -1826,7 +1835,7 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> VhostUserMMap *vu_mmap)
> {
> void *addr = 0;
> - MemoryRegion *mr = NULL;
> + VirtSharedMemory *shmem = NULL;
>
> if (!dev->vdev->shmem_list ||
> dev->vdev->n_shmem_regions <= vu_mmap->shmid) {
> @@ -1836,22 +1845,22 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> return -EFAULT;
> }
>
> - mr = &dev->vdev->shmem_list[vu_mmap->shmid];
> + shmem = &dev->vdev->shmem_list[vu_mmap->shmid];
>
> - if (!mr) {
> + if (!shmem) {
> error_report("VIRTIO Shared Memory Region at "
> "ID %d unitialized", vu_mmap->shmid);
> return -EFAULT;
> }
>
> if ((vu_mmap->shm_offset + vu_mmap->len) < vu_mmap->len ||
> - (vu_mmap->shm_offset + vu_mmap->len) > mr->size) {
> + (vu_mmap->shm_offset + vu_mmap->len) > shmem->mr->size) {
> error_report("Bad offset/len for mmap %" PRIx64 "+%" PRIx64,
> vu_mmap->shm_offset, vu_mmap->len);
> return -EFAULT;
> }
Please add a check for an existing mapping that matches [shm_offset,
shm_offset + len). This will prevent partial unmap as required by the
spec.
>
> - void *shmem_ptr = memory_region_get_ram_ptr(mr);
> + void *shmem_ptr = memory_region_get_ram_ptr(shmem->mr);
>
> addr = mmap(shmem_ptr + vu_mmap->shm_offset, vu_mmap->len,
> PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> @@ -1861,6 +1870,8 @@ vhost_user_backend_handle_shmem_unmap(struct vhost_dev *dev,
> return -EFAULT;
> }
>
> + virtio_del_shmem_map(shmem, vu_mmap->shm_offset, vu_mmap->len);
> +
> return 0;
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ccc4f2cd75..0e2cd62a15 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3059,15 +3059,52 @@ int virtio_save(VirtIODevice *vdev, QEMUFile *f)
> return vmstate_save_state(f, &vmstate_virtio, vdev, NULL);
> }
>
> -MemoryRegion *virtio_new_shmem_region(VirtIODevice *vdev)
> +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev)
> {
> - MemoryRegion *mr;
> + VirtSharedMemory *shmem = NULL;
> ++vdev->n_shmem_regions;
> - vdev->shmem_list = g_renew(MemoryRegion, vdev->shmem_list,
> + vdev->shmem_list = g_renew(VirtSharedMemory, vdev->shmem_list,
> vdev->n_shmem_regions);
> - mr = &vdev->shmem_list[vdev->n_shmem_regions - 1];
> - mr = g_new0(MemoryRegion, 1);
> - return mr;
> + shmem = &vdev->shmem_list[vdev->n_shmem_regions - 1];
> + shmem = g_new0(VirtSharedMemory, 1);
> + QTAILQ_INIT(&shmem->mapped_regions);
> + return shmem;
> +}
> +
> +void virtio_add_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size)
> +{
> + MappedMemoryRegion *mmap = g_new0(MappedMemoryRegion, 1);
> + mmap->offset = offset;
> + mmap->size = int128_make64(size);
> + QTAILQ_REMOVE(&shmem->mapped_regions, mmap, link);
> + g_free(mmap);
mmap needs to be inserted into mapped_regions and must not be freed by
this function.
> +}
> +
> +void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size)
> +{
> + MappedMemoryRegion *mmap = g_new0(MappedMemoryRegion, 1);
> + mmap->offset = offset;
> + mmap->size = int128_make64(size);
> + QTAILQ_INSERT_TAIL(&shmem->mapped_regions, mmap, link);
This function needs to search the list for the matching mmap and remove
it from the list. It should not create a new mmap.
> + g_free(mmap);
> +}
> +
> +bool virtio_shmem_map_overlaps(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size)
> +{
> + MappedMemoryRegion *map_reg;
> + hwaddr new_reg_end = offset + size;
> + QTAILQ_FOREACH(map_reg, &shmem->mapped_regions, link) {
> + hwaddr region_end = map_reg->offset + map_reg->size;
> + if ((map_reg->offset == offset) ||
> + (map_reg->offset < offset && region_end >= offset) ||
> + (offset < map_reg->offset && new_reg_end >= map_reg->offset )) {
> + return true;
> + }
> + }
> + return false;
> }
>
> /* A wrapper for use as a VMState .put function */
> @@ -4007,11 +4044,20 @@ static void virtio_device_free_virtqueues(VirtIODevice *vdev)
> static void virtio_device_instance_finalize(Object *obj)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(obj);
> + VirtSharedMemory *shmem = NULL;
> + int i;
>
> virtio_device_free_virtqueues(vdev);
>
> g_free(vdev->config);
> g_free(vdev->vector_queues);
> + for (i = 0; i< vdev->n_shmem_regions; i++) {
> + shmem = &vdev->shmem_list[i];
> + while (!QTAILQ_EMPTY(&shmem->mapped_regions)) {
> + MappedMemoryRegion *mmap_reg = QTAILQ_FIRST(&shmem->mapped_regions);
> + QTAILQ_REMOVE(&shmem->mapped_regions, mmap_reg, link);
g_free(mmap_reg)
> + }
> + }
> }
>
> static Property virtio_properties[] = {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index d4a2f664d9..5b801f33f5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -98,6 +98,21 @@ enum virtio_device_endian {
> VIRTIO_DEVICE_ENDIAN_BIG,
> };
>
> +struct MappedMemoryRegion {
> + Int128 size;
> + hwaddr offset;
> + QTAILQ_ENTRY(MappedMemoryRegion) link;
> +};
> +
> +typedef struct MappedMemoryRegion MappedMemoryRegion;
> +
> +struct VirtSharedMemory {
> + MemoryRegion *mr;
> + QTAILQ_HEAD(, MappedMemoryRegion) mapped_regions;
> +};
> +
> +typedef struct VirtSharedMemory VirtSharedMemory;
> +
> /**
> * struct VirtIODevice - common VirtIO structure
> * @name: name of the device
> @@ -168,7 +183,7 @@ struct VirtIODevice
> EventNotifier config_notifier;
> bool device_iotlb_enabled;
> /* Shared memory region for vhost-user mappings. */
> - MemoryRegion *shmem_list;
> + VirtSharedMemory *shmem_list;
> int n_shmem_regions;
> };
>
> @@ -289,7 +304,13 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
>
> int virtio_save(VirtIODevice *vdev, QEMUFile *f);
>
> -MemoryRegion *virtio_new_shmem_region(VirtIODevice *vdev);
> +VirtSharedMemory *virtio_new_shmem_region(VirtIODevice *vdev);
> +void virtio_add_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size);
> +void virtio_del_shmem_map(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size);
> +bool virtio_shmem_map_overlaps(VirtSharedMemory *shmem, hwaddr offset,
> + uint64_t size);
>
> extern const VMStateInfo virtio_vmstate_info;
>
> --
> 2.45.2
>
© 2016 - 2024 Red Hat, Inc.