On Fri, 11 Sept 2026 at 10:01, Michael S. Tsirkin <[email protected]> wrote:
>
> When QEMU restores pending virtio requests from a migration stream, it
> calls qemu_get_virtqueue_element(), which calls virtqueue_map() and
> virtqueue_map_iovec() to re-map the saved DMA fragment addresses into
> the destination address space. If the addresses in the migration stream
> are corrupted, and as a result dma_memory_map() returns NULL or a
> shorter-than-expected length, virtqueue_map_iovec() calls exit(1),
> terminating the destination QEMU process instead of failing the
> migration cleanly.
>
> Convert virtqueue_map_iovec() and virtqueue_map() from void to bool.
> On failure, virtqueue_map_iovec() unmaps any entries it has already
> mapped and returns false; virtqueue_map() similarly cleans up the in_sg
> entries if out_sg mapping fails. qemu_get_virtqueue_element() now
> checks the return value, frees the element and returns NULL on failure,
> allowing the migration restore path to propagate a clean error. The
> callers in virtio-blk, virtio-serial-bus and virtio-scsi that invoke
> qemu_get_virtqueue_element() during load are updated to check for NULL
> and return an error code.
>
Hi; Coverity points out an error in the error handling code
added here (CID 1686764):
> -static void virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
> +static bool virtqueue_map_iovec(VirtIODevice *vdev, struct iovec *sg,
> hwaddr *addr, unsigned int num_sg,
> bool is_write)
> {
> unsigned int i;
> hwaddr len;
> + DMADirection dir = is_write ? DMA_DIRECTION_FROM_DEVICE :
> + DMA_DIRECTION_TO_DEVICE;
>
> for (i = 0; i < num_sg; i++) {
> len = sg[i].iov_len;
> - sg[i].iov_base = dma_memory_map(vdev->dma_as,
> - addr[i], &len, is_write ?
> - DMA_DIRECTION_FROM_DEVICE :
> - DMA_DIRECTION_TO_DEVICE,
> - MEMTXATTRS_UNSPECIFIED);
> + sg[i].iov_base = dma_memory_map(vdev->dma_as, addr[i], &len,
> + dir, MEMTXATTRS_UNSPECIFIED);
> if (!sg[i].iov_base) {
> error_report("virtio: error trying to map MMIO memory");
> - exit(1);
> + goto err_undo_map;
If dma_memory_map() fails on the first iteration, we will jump
to err_undo_map, with i == 0....
> }
> if (len != sg[i].iov_len) {
> error_report("virtio: unexpected memory split");
> - exit(1);
> + dma_memory_unmap(vdev->dma_as, sg[i].iov_base, len, dir, 0);
> + goto err_undo_map;
> }
> }
> + return true;
> +
> +err_undo_map:
> + while (i-- > 0) {
...but since 'i' is an unsigned type, that will mean that "i--"
here underflows to an extremely large value. This loop will
then iterate many times unmapping random things.
> + dma_memory_unmap(vdev->dma_as, sg[i].iov_base, sg[i].iov_len,
> + dir, 0);
> + }
> + return false;
> }
thanks
-- PMM
