On Wed, Jul 29, 2026 at 06:52:47PM +0800, Jia Jia wrote:
> Guest MAP and UNMAP requests can set virt_end below virt_start. Since
> virt_end is inclusive, this is not a valid interval. MAP nevertheless
> stores it in domain->mappings, but interval_cmp() assumes low <= high.
> For an inverted key, interval_cmp(key, key) returns -1. A covering UNMAP
> can therefore find the key but fail to remove it and repeat forever while
> holding s->mutex.
>
> Reject inverted request ranges with VIRTIO_IOMMU_S_INVAL and make the
> notifier range decomposition skip invalid ranges. Keep the existing
> notifier-before-remove ordering, but return VIRTIO_IOMMU_S_DEVERR if
> g_tree_remove() fails.
>
> Fixes: fe2cacae2438 ("virtio-iommu: Implement map/unmap")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4104
> Signed-off-by: Jia Jia <[email protected]>
> ---
> v2:
> - Drop migration-state validation and keep the existing post-load callback.
>
> hw/virtio/virtio-iommu.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 533bd5073f..b7145c8277 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -210,7 +210,13 @@ static void
> virtio_iommu_notify_map_unmap(IOMMUMemoryRegion *mr,
> IOMMUTLBEvent *event,
> hwaddr virt_start, hwaddr virt_end)
> {
> - uint64_t delta = virt_end - virt_start;
> + uint64_t delta;
> +
> + if (virt_end < virt_start) {
> + return;
> + }
> +
> + delta = virt_end - virt_start;
you do not need to move the delta assignment.
> event->entry.iova = virt_start;
> event->entry.addr_mask = delta;
> @@ -807,6 +813,10 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
> return VIRTIO_IOMMU_S_INVAL;
> }
>
> + if (virt_end < virt_start) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
> if (!domain) {
> return VIRTIO_IOMMU_S_NOENT;
> @@ -857,6 +867,10 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
>
> trace_virtio_iommu_unmap(domain_id, virt_start, virt_end);
>
> + if (virt_end < virt_start) {
> + return VIRTIO_IOMMU_S_INVAL;
> + }
> +
> domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
> if (!domain) {
> return VIRTIO_IOMMU_S_NOENT;
> @@ -879,7 +893,10 @@ static int virtio_iommu_unmap(VirtIOIOMMU *s,
> virtio_iommu_notify_unmap(ep->iommu_mr, current_low,
> current_high);
> }
> - g_tree_remove(domain->mappings, iter_key);
> + if (!g_tree_remove(domain->mappings, iter_key)) {
> + ret = VIRTIO_IOMMU_S_DEVERR;
> + break;
> + }
> trace_virtio_iommu_unmap_done(domain_id, current_low,
> current_high);
> } else {
> ret = VIRTIO_IOMMU_S_RANGE;
> --
> 2.34.1
