Re: [PATCH 2/7] vfio-container-base: Introduce vfio_container_get_iova_ranges() helper

["Cédric Le Goater" <clg@redhat.com>]

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)