[PATCH for-4.5] libxc: fix mmap leak in xc_unmap_domain_meminfo/xc_map_domain_meminfo

[PATCH for-4.5] libxc: fix mmap leak in xc_unmap_domain_meminfo/xc_map_domain_meminfo

[Vitaly Kuznetsov]

xc_unmap_domain_meminfo uses P2M_FLL_ENTRIES macro instead of P2M_FL_ENTRIES.
Moreover, P2M_FL_ENTRIES macro uses (dinfo->p2m_size) which is always 0 here
as we don't initialize it. The result is that we always unmap just 1 frame.

xc_map_domain_meminfo uses P2M_FLL_ENTRIES macro instead of P2M_FL_ENTRIES
on failure path.

The issue went unnoticed mostly because we use unmap_domain_meminfo and
xc_map_domain_meminfo in one-shot xen-mfndump and xen-hptool (through
xc_exchange_page()) tools. When used is long-running apps (e.g. in xl)
domains become zombies after their death.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 tools/libxc/xc_domain.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 43e1d44..3777b18 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1861,12 +1861,13 @@ int xc_domain_bind_pt_isa_irq(
 
 int xc_unmap_domain_meminfo(xc_interface *xch, struct xc_domain_meminfo *minfo)
 {
-    struct domain_info_context _di = { .guest_width = minfo->guest_width };
+    struct domain_info_context _di = { .guest_width = minfo->guest_width,
+                                       .p2m_size = minfo->p2m_size};
     struct domain_info_context *dinfo = &_di;
 
     free(minfo->pfn_type);
     if ( minfo->p2m_table )
-        munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE);
+        munmap(minfo->p2m_table, P2M_FL_ENTRIES * PAGE_SIZE);
     minfo->p2m_table = NULL;
 
     return 0;
@@ -1977,7 +1978,7 @@ failed:
     }
     if ( minfo->p2m_table )
     {
-        munmap(minfo->p2m_table, P2M_FLL_ENTRIES * PAGE_SIZE);
+        munmap(minfo->p2m_table, P2M_FL_ENTRIES * PAGE_SIZE);
         minfo->p2m_table = NULL;
     }
 
-- 
1.9.3

Re: [PATCH for-4.5] libxc: fix mmap leak in xc_unmap_domain_meminfo/xc_map_domain_meminfo

[Ian Campbell]

On Wed, 2014-10-01 at 15:35 +0200, Vitaly Kuznetsov wrote:
> xc_unmap_domain_meminfo uses P2M_FLL_ENTRIES macro instead of P2M_FL_ENTRIES.
> Moreover, P2M_FL_ENTRIES macro uses (dinfo->p2m_size) which is always 0 here
> as we don't initialize it. The result is that we always unmap just 1 frame.
> 
> xc_map_domain_meminfo uses P2M_FLL_ENTRIES macro instead of P2M_FL_ENTRIES
> on failure path.
> 
> The issue went unnoticed mostly because we use unmap_domain_meminfo and
> xc_map_domain_meminfo in one-shot xen-mfndump and xen-hptool (through
> xc_exchange_page()) tools. When used is long-running apps (e.g. in xl)
> domains become zombies after their death.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>


Good catch. Acked-by: Ian Campbell <ian.campbell@citrix.com> and
applied, thanks!