| Message ID | 20240626082727.1278530-3-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VIRTIO-IOMMU/HostIOMMUDevice: Fixes and page size mask rework | expand |
On 6/26/24 10:26 AM, Eric Auger wrote: > Introduce vfio_container_get_iova_ranges() to retrieve the usable > IOVA regions of the base container and use it in the Host IOMMU > device implementations of get_iova_ranges() callback. > > We also fix a UAF bug as the list was shallow copied while > g_list_free_full() was used both on the single call site, in > virtio_iommu_set_iommu_device() but also in > vfio_container_instance_finalize(). Instead use g_list_copy_deep. > > Fixes: cf2647a76e ("virtio-iommu: Compute host reserved regions") > Signed-off-by: Eric Auger <eric.auger@redhat.com> > Suggested-by: Cédric Le Goater <clg@redhat.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > include/hw/vfio/vfio-container-base.h | 2 ++ > hw/vfio/container-base.c | 15 +++++++++++++++ > hw/vfio/container.c | 8 +------- > hw/vfio/iommufd.c | 8 +------- > 4 files changed, 19 insertions(+), 14 deletions(-) > > diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h > index 419e45ee7a..45d7c40fce 100644 > --- a/include/hw/vfio/vfio-container-base.h > +++ b/include/hw/vfio/vfio-container-base.h > @@ -86,6 +86,8 @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer, > int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer, > VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp); > > +GList *vfio_container_get_iova_ranges(const VFIOContainerBase *bcontainer); > + > #define TYPE_VFIO_IOMMU "vfio-iommu" > #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy" > #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr" > diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c > index 50b1664f89..809b157674 100644 > --- a/hw/vfio/container-base.c > +++ b/hw/vfio/container-base.c > @@ -83,6 +83,21 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer, > errp); > } > > +static gpointer copy_iova_range(gconstpointer src, gpointer data) > +{ > + Range *source = (Range *)src; > + Range *dest = g_new(Range, 1); > + > + range_set_bounds(dest, range_lob(source), range_upb(source)); > + return dest; > +} > + > +GList *vfio_container_get_iova_ranges(const VFIOContainerBase *bcontainer) > +{ > + assert(bcontainer); > + return g_list_copy_deep(bcontainer->iova_ranges, copy_iova_range, NULL); > +} > + > static void vfio_container_instance_finalize(Object *obj) > { > VFIOContainerBase *bcontainer = VFIO_IOMMU(obj); > diff --git a/hw/vfio/container.c b/hw/vfio/container.c > index 2e7ecdf10e..2ad57cd845 100644 > --- a/hw/vfio/container.c > +++ b/hw/vfio/container.c > @@ -1169,15 +1169,9 @@ static GList * > hiod_legacy_vfio_get_iova_ranges(HostIOMMUDevice *hiod, Error **errp) > { > VFIODevice *vdev = hiod->agent; > - GList *l = NULL; > > g_assert(vdev); > - > - if (vdev->bcontainer) { > - l = g_list_copy(vdev->bcontainer->iova_ranges); > - } > - > - return l; > + return vfio_container_get_iova_ranges(vdev->bcontainer); > } > > static void vfio_iommu_legacy_instance_init(Object *obj) > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index c2f158e603..890d8d6a38 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -647,15 +647,9 @@ static GList * > hiod_iommufd_vfio_get_iova_ranges(HostIOMMUDevice *hiod, Error **errp) > { > VFIODevice *vdev = hiod->agent; > - GList *l = NULL; > > g_assert(vdev); > - > - if (vdev->bcontainer) { > - l = g_list_copy(vdev->bcontainer->iova_ranges); > - } > - > - return l; > + return vfio_container_get_iova_ranges(vdev->bcontainer); > } > > static void hiod_iommufd_vfio_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h index 419e45ee7a..45d7c40fce 100644 --- a/include/hw/vfio/vfio-container-base.h +++ b/include/hw/vfio/vfio-container-base.h @@ -86,6 +86,8 @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer, int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer, VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp); +GList *vfio_container_get_iova_ranges(const VFIOContainerBase *bcontainer); + #define TYPE_VFIO_IOMMU "vfio-iommu" #define TYPE_VFIO_IOMMU_LEGACY TYPE_VFIO_IOMMU "-legacy" #define TYPE_VFIO_IOMMU_SPAPR TYPE_VFIO_IOMMU "-spapr" diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c index 50b1664f89..809b157674 100644 --- a/hw/vfio/container-base.c +++ b/hw/vfio/container-base.c @@ -83,6 +83,21 @@ int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer, errp); } +static gpointer copy_iova_range(gconstpointer src, gpointer data) +{ + Range *source = (Range *)src; + Range *dest = g_new(Range, 1); + + range_set_bounds(dest, range_lob(source), range_upb(source)); + return dest; +} + +GList *vfio_container_get_iova_ranges(const VFIOContainerBase *bcontainer) +{ + assert(bcontainer); + return g_list_copy_deep(bcontainer->iova_ranges, copy_iova_range, NULL); +} + static void vfio_container_instance_finalize(Object *obj) { VFIOContainerBase *bcontainer = VFIO_IOMMU(obj); diff --git a/hw/vfio/container.c b/hw/vfio/container.c index 2e7ecdf10e..2ad57cd845 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -1169,15 +1169,9 @@ static GList * hiod_legacy_vfio_get_iova_ranges(HostIOMMUDevice *hiod, Error **errp) { VFIODevice *vdev = hiod->agent; - GList *l = NULL; g_assert(vdev); - - if (vdev->bcontainer) { - l = g_list_copy(vdev->bcontainer->iova_ranges); - } - - return l; + return vfio_container_get_iova_ranges(vdev->bcontainer); } static void vfio_iommu_legacy_instance_init(Object *obj) diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index c2f158e603..890d8d6a38 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -647,15 +647,9 @@ static GList * hiod_iommufd_vfio_get_iova_ranges(HostIOMMUDevice *hiod, Error **errp) { VFIODevice *vdev = hiod->agent; - GList *l = NULL; g_assert(vdev); - - if (vdev->bcontainer) { - l = g_list_copy(vdev->bcontainer->iova_ranges); - } - - return l; + return vfio_container_get_iova_ranges(vdev->bcontainer); } static void hiod_iommufd_vfio_class_init(ObjectClass *oc, void *data)
Introduce vfio_container_get_iova_ranges() to retrieve the usable IOVA regions of the base container and use it in the Host IOMMU device implementations of get_iova_ranges() callback. We also fix a UAF bug as the list was shallow copied while g_list_free_full() was used both on the single call site, in virtio_iommu_set_iommu_device() but also in vfio_container_instance_finalize(). Instead use g_list_copy_deep. Fixes: cf2647a76e ("virtio-iommu: Compute host reserved regions") Signed-off-by: Eric Auger <eric.auger@redhat.com> Suggested-by: Cédric Le Goater <clg@redhat.com> --- include/hw/vfio/vfio-container-base.h | 2 ++ hw/vfio/container-base.c | 15 +++++++++++++++ hw/vfio/container.c | 8 +------- hw/vfio/iommufd.c | 8 +------- 4 files changed, 19 insertions(+), 14 deletions(-)