From 429c72800654503e0073906f63fdc9a641639bdc Mon Sep 17 00:00:00 2001 From: Kunkun Jiang Date: Mon, 11 Jul 2022 09:46:51 +0800 Subject: [PATCH 1/2] vfio/migration: Fix incorrect initialization value for parameters in VFIOMigration The structure VFIOMigration of a VFIODevice is allocated and initialized in vfio_migration_init(). "device_state" and "vm_running" are initialized to 0, indicating that VFIO device is_STOP and VM is not-running. The initialization value is incorrect. According to the agreement, default state of VFIO device is _RUNNING. And if a VFIO device is hot-plugged while the VM is running, "vm_running" should be 1. This patch fixes it. Fixes: 02a7e71b1e5b ("vfio: Add VM state change handler to know state of VM") Signed-off-by: Kunkun Jiang Link: https://lore.kernel.org/r/20220711014651.1327-1-jiangkunkun@huawei.com Signed-off-by: Alex Williamson --- hw/vfio/migration.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index a6ad1f8945..3de4252111 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -806,6 +806,8 @@ static int vfio_migration_init(VFIODevice *vbasedev, } vbasedev->migration = g_new0(VFIOMigration, 1); + vbasedev->migration->device_state = VFIO_DEVICE_STATE_RUNNING; + vbasedev->migration->vm_running = runstate_is_running(); ret = vfio_region_setup(obj, vbasedev, &vbasedev->migration->region, info->index, "migration"); From 85b6d2b5fc25c9c0d10d493b3728183ab8f8e68a Mon Sep 17 00:00:00 2001 From: Alex Williamson Date: Thu, 15 Sep 2022 11:18:27 -0600 Subject: [PATCH 2/2] vfio/common: Fix vfio_iommu_type1_info use after free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On error, vfio_get_iommu_info() frees and clears *info, but vfio_connect_container() continues to use the pointer regardless of the return value. Restructure the code such that a failure of this function triggers an error and clean up the remainder of the function, including updating an outdated comment that had drifted from its relevant line of code and using host page size for a default for better compatibility on non-4KB systems. Reported-by: Nicolin Chen Link: https://lore.kernel.org/all/20220910004245.2878-1-nicolinc@nvidia.com/ Signed-off-by: Alex Williamson Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Nicolin Chen Tested-by: Nicolin Chen Link: https://lore.kernel.org/r/166326219630.3388898.12882473157184946072.stgit@omen Signed-off-by: Alex Williamson --- hw/vfio/common.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ace9562a9b..6b5d8c0bf6 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -2111,29 +2111,31 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, { struct vfio_iommu_type1_info *info; - /* - * FIXME: This assumes that a Type1 IOMMU can map any 64-bit - * IOVA whatsoever. That's not actually true, but the current - * kernel interface doesn't tell us what it can map, and the - * existing Type1 IOMMUs generally support any IOVA we're - * going to actually try in practice. - */ ret = vfio_get_iommu_info(container, &info); - - if (ret || !(info->flags & VFIO_IOMMU_INFO_PGSIZES)) { - /* Assume 4k IOVA page size */ - info->iova_pgsizes = 4096; + if (ret) { + error_setg_errno(errp, -ret, "Failed to get VFIO IOMMU info"); + goto enable_discards_exit; } - vfio_host_win_add(container, 0, (hwaddr)-1, info->iova_pgsizes); - container->pgsizes = info->iova_pgsizes; - /* The default in the kernel ("dma_entry_limit") is 65535. */ - container->dma_max_mappings = 65535; - if (!ret) { - vfio_get_info_dma_avail(info, &container->dma_max_mappings); - vfio_get_iommu_info_migration(container, info); + if (info->flags & VFIO_IOMMU_INFO_PGSIZES) { + container->pgsizes = info->iova_pgsizes; + } else { + container->pgsizes = qemu_real_host_page_size(); } + + if (!vfio_get_info_dma_avail(info, &container->dma_max_mappings)) { + container->dma_max_mappings = 65535; + } + vfio_get_iommu_info_migration(container, info); g_free(info); + + /* + * FIXME: We should parse VFIO_IOMMU_TYPE1_INFO_CAP_IOVA_RANGE + * information to get the actual window extent rather than assume + * a 64-bit IOVA address space. + */ + vfio_host_win_add(container, 0, (hwaddr)-1, container->pgsizes); + break; } case VFIO_SPAPR_TCE_v2_IOMMU: