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@vates.tech>
---
 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;
-- 
2.45.2



Teddy Astie | Vates XCP-ng Intern

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

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@vates.tech>
> ---
>  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