[PATCH v1 0/2] Misc fixes regarding releasing resources on ARM

[PATCH v1 0/2] Misc fixes regarding releasing resources on ARM

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Oleksandr Tyshchenko (2):
  xen/arm: vgic: Check for vgic handler to be initialized before
    dereferencing it
  xen/arm: p2m: Check for p2m->domain to be initialized before releasing
    resources

 xen/arch/arm/p2m.c  | 13 ++++++++++++-
 xen/arch/arm/vgic.c |  3 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v1 1/2] xen/arm: vgic: Check for vgic handler to be initialized before dereferencing it

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Since domain_vgic_free() can be called when the vgic_ops haven't been
initialised yet, always check that d->arch.vgic.handler is not a null.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
 xen/arch/arm/vgic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7a4e3cd..6cf947c 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -187,7 +187,8 @@ void domain_vgic_free(struct domain *d)
         }
     }
 
-    d->arch.vgic.handler->domain_free(d);
+    if ( d->arch.vgic.handler )
+       d->arch.vgic.handler->domain_free(d);
     xfree(d->arch.vgic.shared_irqs);
     xfree(d->arch.vgic.pending_irqs);
     xfree(d->arch.vgic.allocated_irqs);
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v1 2/2] xen/arm: p2m: Check for p2m->domain to be initialized before releasing resources

[Oleksandr Tyshchenko]

From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Since p2m_teardown() can be called when p2m_init() haven't executed yet
we might deal with unitialized list "p2m->pages" which leads to crash.
To avoid this use back pointer to domain as end-of-initialization indicator.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
 xen/arch/arm/p2m.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c484469..141ae7e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1219,6 +1219,9 @@ void p2m_teardown(struct domain *d)
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     struct page_info *pg;
 
+    if ( !p2m->domain )
+        return;
+
     while ( (pg = page_list_remove_head(&p2m->pages)) )
         free_domheap_page(pg);
 
@@ -1230,6 +1233,8 @@ void p2m_teardown(struct domain *d)
     p2m_free_vmid(d);
 
     radix_tree_destroy(&p2m->mem_access_settings, NULL);
+
+    p2m->domain = NULL;
 }
 
 int p2m_init(struct domain *d)
@@ -1247,7 +1252,6 @@ int p2m_init(struct domain *d)
     if ( rc != 0 )
         return rc;
 
-    p2m->domain = d;
     p2m->max_mapped_gfn = _gfn(0);
     p2m->lowest_mapped_gfn = _gfn(ULONG_MAX);
 
@@ -1276,6 +1280,13 @@ int p2m_init(struct domain *d)
     for_each_possible_cpu(cpu)
        p2m->last_vcpu_ran[cpu] = INVALID_VCPU_ID;
 
+    /*
+     * Besides getting a domain when we only have the p2m in hand,
+     * the back pointer to domain is also used in p2m_teardown()
+     * as an end-of-initialization indicator.
+     */
+    p2m->domain = d;
+
     return rc;
 }
 
-- 
2.7.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1 0/2] Misc fixes regarding releasing resources on ARM

[Oleksandr Tyshchenko]

ping

On Mon, Aug 28, 2017 at 8:32 PM, Oleksandr Tyshchenko
<olekstysh@gmail.com> wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> Oleksandr Tyshchenko (2):
>   xen/arm: vgic: Check for vgic handler to be initialized before
>     dereferencing it
>   xen/arm: p2m: Check for p2m->domain to be initialized before releasing
>     resources
>
>  xen/arch/arm/p2m.c  | 13 ++++++++++++-
>  xen/arch/arm/vgic.c |  3 ++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> --
> 2.7.4
>



-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1 0/2] Misc fixes regarding releasing resources on ARM

[Julien Grall]

Hi Oleksandr,

On 06/09/17 10:16, Oleksandr Tyshchenko wrote:
> ping

Sorry I was on holidays, I will try to have a look today.

Cheers,

> 
> On Mon, Aug 28, 2017 at 8:32 PM, Oleksandr Tyshchenko
> <olekstysh@gmail.com> wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> Oleksandr Tyshchenko (2):
>>    xen/arm: vgic: Check for vgic handler to be initialized before
>>      dereferencing it
>>    xen/arm: p2m: Check for p2m->domain to be initialized before releasing
>>      resources
>>
>>   xen/arch/arm/p2m.c  | 13 ++++++++++++-
>>   xen/arch/arm/vgic.c |  3 ++-
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> --
>> 2.7.4
>>
> 
> 
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1 1/2] xen/arm: vgic: Check for vgic handler to be initialized before dereferencing it

[Stefano Stabellini]

On Mon, 28 Aug 2017, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Since domain_vgic_free() can be called when the vgic_ops haven't been
> initialised yet, always check that d->arch.vgic.handler is not a null.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

There is a code style problem, but I'll fix on commit.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/vgic.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 7a4e3cd..6cf947c 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -187,7 +187,8 @@ void domain_vgic_free(struct domain *d)
>          }
>      }
>  
> -    d->arch.vgic.handler->domain_free(d);
> +    if ( d->arch.vgic.handler )
> +       d->arch.vgic.handler->domain_free(d);
>      xfree(d->arch.vgic.shared_irqs);
>      xfree(d->arch.vgic.pending_irqs);
>      xfree(d->arch.vgic.allocated_irqs);
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1 2/2] xen/arm: p2m: Check for p2m->domain to be initialized before releasing resources

[Stefano Stabellini]

On Mon, 28 Aug 2017, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> 
> Since p2m_teardown() can be called when p2m_init() haven't executed yet
> we might deal with unitialized list "p2m->pages" which leads to crash.
> To avoid this use back pointer to domain as end-of-initialization indicator.
> 
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> ---
>  xen/arch/arm/p2m.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index c484469..141ae7e 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -1219,6 +1219,9 @@ void p2m_teardown(struct domain *d)
>      struct p2m_domain *p2m = p2m_get_hostp2m(d);
>      struct page_info *pg;

The patch looks good. I'll add a comment saying "p2m not actually
initialized" here.

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> +    if ( !p2m->domain )
> +        return;
> +
>      while ( (pg = page_list_remove_head(&p2m->pages)) )
>          free_domheap_page(pg);
>  
> @@ -1230,6 +1233,8 @@ void p2m_teardown(struct domain *d)
>      p2m_free_vmid(d);
>  
>      radix_tree_destroy(&p2m->mem_access_settings, NULL);
> +
> +    p2m->domain = NULL;
>  }
>  
>  int p2m_init(struct domain *d)
> @@ -1247,7 +1252,6 @@ int p2m_init(struct domain *d)
>      if ( rc != 0 )
>          return rc;
>  
> -    p2m->domain = d;
>      p2m->max_mapped_gfn = _gfn(0);
>      p2m->lowest_mapped_gfn = _gfn(ULONG_MAX);
>  
> @@ -1276,6 +1280,13 @@ int p2m_init(struct domain *d)
>      for_each_possible_cpu(cpu)
>         p2m->last_vcpu_ran[cpu] = INVALID_VCPU_ID;
>  
> +    /*
> +     * Besides getting a domain when we only have the p2m in hand,
> +     * the back pointer to domain is also used in p2m_teardown()
> +     * as an end-of-initialization indicator.
> +     */
> +    p2m->domain = d;
> +
>      return rc;
>  }
>  
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v1 0/2] Misc fixes regarding releasing resources on ARM

[Stefano Stabellini]

On Mon, 28 Aug 2017, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

Thanks for the fixes, I committed the two patches.


> Oleksandr Tyshchenko (2):
>   xen/arm: vgic: Check for vgic handler to be initialized before
>     dereferencing it
>   xen/arm: p2m: Check for p2m->domain to be initialized before releasing
>     resources
> 
>  xen/arch/arm/p2m.c  | 13 ++++++++++++-
>  xen/arch/arm/vgic.c |  3 ++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel