References: bnc#823011 XSA-55

From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Thu, 13 Jun 2013 19:10:15 +0100
Subject: [PATCH 18/21] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range

The return values from xc_dom_*_to_ptr and xc_map_foreign_range are
sometimes dereferenced, or subjected to pointer arithmetic, without
checking whether the relevant function failed and returned NULL.

Add an appropriate error check at every call site.

Changes in the 4.2 backport of this series:
* Fix tools/libxc/xc_dom_x86.c:setup_pgtables_x86_32.
* Fix tools/libxc/xc_dom_ia64.c:start_info_ia64.
* Fix tools/libxc/ia64/xc_ia64_dom_fwloader.c:xc_dom_load_fw_kernel.

Conflicts in the 4.1 backport of this series:
* xc_dom_load_elf_kernel has less error handling in 4.1.
* the VM generation ID code is not in 4.1.

This is part of the fix to a security issue, XSA-55.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

--- a/tools/libxc/ia64/xc_ia64_dom_fwloader.c
+++ b/tools/libxc/ia64/xc_ia64_dom_fwloader.c
@@ -44,6 +44,8 @@ static int xc_dom_load_fw_kernel(struct 
     unsigned long i;
 
     dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart);
+    if ( dest == NULL )
+        return -1;
     memcpy(dest, dom->kernel_blob, FW_SIZE);
 
     /* Synchronize cache.  */
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -260,6 +260,12 @@ static int xc_dom_load_bin_kernel(struct
     xc_dom_printf("  bss_size:  0x%" PRIx32 "\n", bss_size);
 
     dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart, &dest_size);
+    if ( dest == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart)"
+                      " => NULL\n", __FUNCTION__);
+        return -EINVAL;
+    }
 
     if ( dest_size < text_size ||
          dest_size - text_size < bss_size )
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -843,6 +843,12 @@ int xc_dom_build_image(struct xc_dom_ima
                                   ramdisklen) != 0 )
             goto err;
         ramdiskmap = xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg);
+        if ( ramdiskmap == NULL )
+        {
+            xc_dom_printf("%s: xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg) => NULL\n",
+                          __FUNCTION__);
+            goto err;
+        }
         if ( unziplen )
         {
             if ( xc_dom_do_gunzip(dom->ramdisk_blob, dom->ramdisk_size,
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -104,6 +104,12 @@ static elf_errorstatus xc_dom_load_elf_s
             return 0;
         size = dom->kernel_seg.vend - dom->bsd_symtab_start;
         hdr_ptr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start, &allow_size);
+        if ( hdr_ptr == NULL )
+        {
+            xc_dom_printf("%s/load: xc_dom_vaddr_to_ptr(dom,dom->bsd_symtab_start"
+                          " => NULL\n", __FUNCTION__);
+            return -1;
+        }
         elf->caller_xdest_base = hdr_ptr;
         elf->caller_xdest_size = allow_size;
         hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
@@ -342,6 +348,12 @@ static elf_errorstatus xc_dom_load_elf_k
     xen_pfn_t pages;
 
     elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg, &pages);
+    if ( elf->dest_base == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_vaddr_to_ptr(dom,dom->kernel_seg)"
+                      " => NULL\n", __FUNCTION__);
+        return -1;
+    }
     elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom);
     elf_load_binary(elf);
     if ( dom->parms.bsd_symtab )
--- a/tools/libxc/xc_dom_ia64.c
+++ b/tools/libxc/xc_dom_ia64.c
@@ -46,6 +46,12 @@ int start_info_ia64(struct xc_dom_image 
 
     xc_dom_printf("%s\n", __FUNCTION__);
 
+    if ( start_info == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_pfn_to_ptr failed on start_info\n", __FUNCTION__);
+        return -1; /* our caller throws away our return value :-/ */
+    }
+
     memset(start_info, 0, sizeof(*start_info));
     sprintf(start_info->magic, dom->guest_type);
     start_info->flags = dom->flags;
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -140,6 +140,9 @@ static int setup_pgtables_x86_32(struct 
     xen_vaddr_t addr;
     xen_pfn_t pgpfn;
 
+    if ( l2tab == NULL )
+        goto pfn_error;
+
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
           addr += PAGE_SIZE_X86 )
     {
@@ -147,6 +150,8 @@ static int setup_pgtables_x86_32(struct 
         {
             /* get L1 tab, make L2 entry */
             l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+            if ( l1tab == NULL )
+                goto pfn_error;
             l2off = l2_table_offset_i386(addr);
             l2tab[l2off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -165,6 +170,11 @@ static int setup_pgtables_x86_32(struct 
             l1tab = NULL;
     }
     return 0;
+
+pfn_error:
+    xc_dom_panic(XC_INTERNAL_ERROR,
+                 "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+    return -EINVAL;
 }
 
 /*
@@ -216,6 +226,12 @@ static xen_pfn_t move_l3_below_4G(struct
         goto out;
 
     l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+    if ( l3tab == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_pfn_to_ptr(dom, l3pfn, 1) => NULL\n",
+                      __FUNCTION__);
+        return l3mfn; /* our one call site will call xc_dom_panic and fail */
+    }
     memset(l3tab, 0, XC_DOM_PAGE_SIZE(dom));
 
     xc_dom_printf("%s: successfully relocated L3 below 4G. "
@@ -259,6 +275,8 @@ static int setup_pgtables_x86_32_pae(str
     }
 
     l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+    if ( l3tab == NULL )
+        goto pfn_error;
 
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
           addr += PAGE_SIZE_X86 )
@@ -267,6 +285,8 @@ static int setup_pgtables_x86_32_pae(str
         {
             /* get L2 tab, make L3 entry */
             l2tab = xc_dom_pfn_to_ptr(dom, l2pfn, 1);
+            if ( l2tab == NULL )
+                goto pfn_error;
             l3off = l3_table_offset_pae(addr);
             l3tab[l3off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
@@ -277,6 +297,8 @@ static int setup_pgtables_x86_32_pae(str
         {
             /* get L1 tab, make L2 entry */
             l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+            if ( l1tab == NULL )
+                goto pfn_error;
             l2off = l2_table_offset_pae(addr);
             l2tab[l2off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -303,6 +325,11 @@ static int setup_pgtables_x86_32_pae(str
         l3tab[3] = pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
     }
     return 0;
+
+pfn_error:
+    xc_dom_panic(XC_INTERNAL_ERROR,
+                 "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+    return -EINVAL;
 }
 
 #undef L1_PROT
@@ -341,6 +368,9 @@ static int setup_pgtables_x86_64(struct 
     uint64_t addr;
     xen_pfn_t pgpfn;
 
+    if ( l4tab == NULL )
+        goto pfn_error;
+
     for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
           addr += PAGE_SIZE_X86 )
     {
@@ -348,6 +378,8 @@ static int setup_pgtables_x86_64(struct 
         {
             /* get L3 tab, make L4 entry */
             l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+            if ( l3tab == NULL )
+                goto pfn_error;
             l4off = l4_table_offset_x86_64(addr);
             l4tab[l4off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l3pfn)) | L4_PROT;
@@ -358,6 +390,8 @@ static int setup_pgtables_x86_64(struct 
         {
             /* get L2 tab, make L3 entry */
             l2tab = xc_dom_pfn_to_ptr(dom, l2pfn, 1);
+            if ( l2tab == NULL )
+                goto pfn_error;
             l3off = l3_table_offset_x86_64(addr);
             l3tab[l3off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
@@ -370,6 +404,8 @@ static int setup_pgtables_x86_64(struct 
         {
             /* get L1 tab, make L2 entry */
             l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+            if ( l1tab == NULL )
+                goto pfn_error;
             l2off = l2_table_offset_x86_64(addr);
             l2tab[l2off] =
                 pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -390,6 +426,11 @@ static int setup_pgtables_x86_64(struct 
             l1tab = NULL;
     }
     return 0;
+
+pfn_error:
+    xc_dom_panic(XC_INTERNAL_ERROR,
+                 "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+    return -EINVAL;
 }
 
 #undef L1_PROT
@@ -407,6 +448,8 @@ static int alloc_magic_pages(struct xc_d
     if ( xc_dom_alloc_segment(dom, &dom->p2m_seg, "phys2mach", 0, p2m_size) )
         return -1;
     dom->p2m_guest = xc_dom_seg_to_ptr(dom, &dom->p2m_seg);
+    if ( dom->p2m_guest == NULL )
+        return -1;
 
     /* allocate special pages */
     dom->start_info_pfn = xc_dom_alloc_page(dom, "start info");
@@ -431,6 +474,12 @@ static int start_info_x86_32(struct xc_d
 
     xc_dom_printf("%s: called\n", __FUNCTION__);
 
+    if ( start_info == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_pfn_to_ptr failed on start_info\n", __FUNCTION__);
+        return -1; /* our caller throws away our return value :-/ */
+    }
+
     memset(start_info, 0, sizeof(*start_info));
     strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
     start_info->magic[sizeof(start_info->magic) - 1] = '\0';
@@ -471,6 +520,12 @@ static int start_info_x86_64(struct xc_d
 
     xc_dom_printf("%s: called\n", __FUNCTION__);
 
+    if ( start_info == NULL )
+    {
+        xc_dom_printf("%s: xc_dom_pfn_to_ptr failed on start_info\n", __FUNCTION__);
+        return -1; /* our caller throws away our return value :-/ */
+    }
+
     memset(start_info, 0, sizeof(*start_info));
     strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
     start_info->magic[sizeof(start_info->magic) - 1] = '\0';
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1313,6 +1313,11 @@ int xc_domain_restore(int xc_handle, int
                 l3tab = (uint64_t *)
                     xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,
                                          PROT_READ, ctx->p2m[i]);
+                if ( l3tab == NULL )
+                {
+                    PERROR("xc_map_foreign_range failed (for l3tab)");
+                    goto out;
+                }
 
                 for ( j = 0; j < 4; j++ )
                     l3ptes[j] = l3tab[j];
@@ -1339,6 +1344,11 @@ int xc_domain_restore(int xc_handle, int
                 l3tab = (uint64_t *)
                     xc_map_foreign_range(xc_handle, dom, PAGE_SIZE,
                                          PROT_READ | PROT_WRITE, ctx->p2m[i]);
+                if ( l3tab == NULL )
+                {
+                    PERROR("xc_map_foreign_range failed (for l3tab, 2nd)");
+                    goto out;
+                }
 
                 for ( j = 0; j < 4; j++ )
                     l3tab[j] = l3ptes[j];
@@ -1524,6 +1534,12 @@ int xc_domain_restore(int xc_handle, int
             SET_FIELD(&ctxt, user_regs.edx, mfn);
             start_info = xc_map_foreign_range(
                 xc_handle, dom, PAGE_SIZE, PROT_READ | PROT_WRITE, mfn);
+            if ( start_info == NULL )
+            {
+                PERROR("xc_map_foreign_range failed (for start_info)");
+                goto out;
+            }
+
             SET_FIELD(start_info, nr_pages, dinfo->p2m_size);
             SET_FIELD(start_info, shared_info, shared_info_frame<<PAGE_SHIFT);
             SET_FIELD(start_info, flags, 0);
@@ -1626,6 +1642,11 @@ int xc_domain_restore(int xc_handle, int
     /* Restore contents of shared-info page. No checking needed. */
     new_shared_info = xc_map_foreign_range(
         xc_handle, dom, PAGE_SIZE, PROT_WRITE, shared_info_frame);
+    if ( new_shared_info == NULL )
+    {
+        PERROR("xc_map_foreign_range failed (for new_shared_info)");
+        goto out;
+    }
 
     /* restore saved vcpu_info and arch specific info */
     MEMCPY_FIELD(new_shared_info, old_shared_info, vcpu_info);
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -730,6 +730,11 @@ int xc_exchange_page(int xc_handle, int 
 
         new_p = xc_map_foreign_range(xc_handle, domid, PAGE_SIZE,
                                      PROT_READ|PROT_WRITE, new_mfn);
+        if ( new_p == NULL )
+        {
+            ERROR("failed to map new_p for copy, guest may be broken?");
+            goto failed;
+        }
         memcpy(new_p, backup, PAGE_SIZE);
         munmap(new_p, PAGE_SIZE);
         mops.arg1.mfn = new_mfn;
