Re: [PATCH for-4.19?] x86/IOMMU: Move allocation in iommu_identity_mapping

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Wed Jul 17, 2024 at 4:51 PM BST, Teddy Astie wrote:
> If for some reason, xmalloc fails after having mapped the
> reserved regions, a error is reported, but the regions are
> actually mapped in p2m.
>
> Move the allocation before trying to map the regions, in
> case the allocation fails, no mapping is actually done
> which could allows this operation to be retried with the
> same regions without failing due to already existing mappings.
>
> Fixes: c0e19d7c6c ("IOMMU: generalize VT-d's tracking of mapped RMRR regions")
> Signed-off-by: Teddy Astie <teddy.astie@xxxxxxxxxx>
> ---
>  xen/drivers/passthrough/x86/iommu.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/x86/iommu.c 
> b/xen/drivers/passthrough/x86/iommu.c
> index cc0062b027..b6bc6d71cb 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -267,18 +267,22 @@ int iommu_identity_mapping(struct domain *d, 
> p2m_access_t p2ma,
>      if ( p2ma == p2m_access_x )
>          return -ENOENT;
>  
> +    map = xmalloc(struct identity_map);
> +    if ( !map )
> +        return -ENOMEM;
> +
>      while ( base_pfn < end_pfn )
>      {
>          int err = set_identity_p2m_entry(d, base_pfn, p2ma, flag);
>  
>          if ( err )
> +        {
> +            xfree(map);
>              return err;
> +        }
>          base_pfn++;
>      }
>  
> -    map = xmalloc(struct identity_map);
> -    if ( !map )
> -        return -ENOMEM;
>      map->base = base;
>      map->end = end;
>      map->access = p2ma;

That covers the case where xmalloc fails, but what about the case where
set_identity_p2m_entry() fails in for a middle pfn? (i.e: due to ENOMEM).

Cheers,
Alejandro