libelf: Make all callers call elf_check_broken

This arranges that if the new pointer reference error checking
tripped, we actually get a message about it.  In this patch these
messages do not change the actual return values from the various
functions: so pointer reference errors do not prevent loading.  This
is for fear that some existing kernels might cause the code to make
these wild references, which would then break, which is not a good
thing in a security patch.

In xen/arch/x86/domain_build.c we have to introduce an "out" label and
change all of the "return rc" beyond the relevant point into "goto
out".

Difference in the 4.2 series, compared to unstable:

* tools/libxc/xc_hvm_build_x86.c:setup_guest and
  xen/arch/arm/kernel.c:kernel_try_elf_prepare have different
  error handling in 4.2 to unstable; patch adjusted accordingly.

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

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

xen-unstable version Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

5 files changed, 64 insertions, 12 deletions

diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index ea458864f0..4fb4da23a7 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -276,6 +276,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
             elf_store_field(elf, shdr, e32.sh_name, 0);
     }
 
+    if ( elf_check_broken(&syms) )
+        DOMPRINTF("%s: symbols ELF broken: %s", __FUNCTION__,
+                  elf_check_broken(&syms));
+    if ( elf_check_broken(elf) )
+        DOMPRINTF("%s: ELF broken: %s", __FUNCTION__,
+                  elf_check_broken(elf));
+
     if ( tables == 0 )
     {
         DOMPRINTF("%s: no symbol table present", __FUNCTION__);
@@ -312,19 +319,23 @@ static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
     {
         xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: ELF image"
                      " has no shstrtab", __FUNCTION__);
-        return -EINVAL;
+        rc = -EINVAL;
+        goto out;
     }
 
     /* parse binary and get xen meta info */
     elf_parse_binary(elf);
     if ( (rc = elf_xen_parse(elf, &dom->parms)) != 0 )
-        return rc;
+    {
+        goto out;
+    }
 
     if ( elf_xen_feature_get(XENFEAT_dom0, dom->parms.f_required) )
     {
         xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: Kernel does not"
                      " support unprivileged (DomU) operation", __FUNCTION__);
-        return -EINVAL;
+        rc = -EINVAL;
+        goto out;
     }
 
     /* find kernel segment */
@@ -338,7 +349,13 @@ static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
     DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
               __FUNCTION__, dom->guest_type,
               dom->kernel_seg.vstart, dom->kernel_seg.vend);
-    return 0;
+    rc = 0;
+out:
+    if ( elf_check_broken(elf) )
+        DOMPRINTF("%s: ELF broken: %s", __FUNCTION__,
+                  elf_check_broken(elf));
+
+    return rc;
 }
 
 static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index ccfd8b5458..8165287a9a 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -403,11 +403,16 @@ static int setup_guest(xc_interface *xch,
         munmap(page0, PAGE_SIZE);
     }
 
+    if ( elf_check_broken(&elf) )
+        ERROR("HVM ELF broken: %s", elf_check_broken(&elf));
+
     free(page_array);
     return 0;
 
  error_out:
     free(page_array);
+    if ( elf_check_broken(&elf) )
+        ERROR("HVM ELF broken, failing: %s", elf_check_broken(&elf));
     return -1;
 }
 
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index cfae99486c..d1f7a30f73 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -301,6 +301,9 @@ int main(int argc, char **argv)
 		printf("__xen_guest: %s\n",
                        elf_strfmt(&elf, elf_section_start(&elf, shdr)));
 
+	if (elf_check_broken(&elf))
+		printf("warning: broken ELF: %s\n", elf_check_broken(&elf));
+
 	return 0;
 }
 
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 2d56130354..dec051938c 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -146,6 +146,8 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
 {
     int rc;
 
+    memset(&info->elf.elf, 0, sizeof(info->elf.elf));
+
     info->kernel_order = get_order_from_bytes(KERNEL_FLASH_SIZE);
     info->kernel_img = alloc_xenheap_pages(info->kernel_order, 0);
     if ( info->kernel_img == NULL )
@@ -160,7 +162,7 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
 #endif
     elf_parse_binary(&info->elf.elf);
     if ( (rc = elf_xen_parse(&info->elf.elf, &info->elf.parms)) != 0 )
-        return rc;
+        goto err;
 
     /*
      * TODO: can the ELF header be used to find the physical address
@@ -169,7 +171,18 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
     info->entry = info->elf.parms.virt_entry;
     info->load = kernel_elf_load;
 
+    if ( elf_check_broken(&info->elf.elf) )
+        printk("Xen: warning: ELF kernel broken: %s\n",
+               elf_check_broken(&info->elf.elf));
+
     return 0;
+
+err:
+    if ( elf_check_broken(&info->elf.elf) )
+        printk("Xen: ELF kernel broken: %s\n",
+               elf_check_broken(&info->elf.elf));
+
+    return rc;
 }
 
 int kernel_prepare(struct kernel_info *info)
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index a655b21b8f..0dbec96b05 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -374,7 +374,7 @@ int __init construct_dom0(
 #endif
     elf_parse_binary(&elf);
     if ( (rc = elf_xen_parse(&elf, &parms)) != 0 )
-        return rc;
+        goto out;
 
     /* compatibility check */
     compatible = 0;
@@ -413,14 +413,16 @@ int __init construct_dom0(
     if ( !compatible )
     {
         printk("Mismatch between Xen and DOM0 kernel\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto out;
     }
 
     if ( parms.elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type != XEN_ENT_NONE &&
          !test_bit(XENFEAT_dom0, parms.f_supported) )
     {
         printk("Kernel does not support Dom0 operation\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto out;
     }
 
 #if defined(__x86_64__)
@@ -734,7 +736,8 @@ int __init construct_dom0(
          (v_end > HYPERVISOR_COMPAT_VIRT_START(d)) )
     {
         printk("DOM0 image overlaps with Xen private area.\n");
-        return -EINVAL;
+        rc = -EINVAL;
+        goto out;
     }
 
     if ( is_pv_32on64_domain(d) )
@@ -914,7 +917,7 @@ int __init construct_dom0(
     if ( rc < 0 )
     {
         printk("Failed to load the kernel binary\n");
-        return rc;
+        goto out;
     }
     bootstrap_map(NULL);
 
@@ -925,7 +928,8 @@ int __init construct_dom0(
         {
             write_ptbase(current);
             printk("Invalid HYPERCALL_PAGE field in ELF notes.\n");
-            return -1;
+            rc = -1;
+            goto out;
         }
         hypercall_page_initialise(
             d, (void *)(unsigned long)parms.virt_hypercall);
@@ -1272,9 +1276,19 @@ int __init construct_dom0(
 
     BUG_ON(rc != 0);
 
-    iommu_dom0_init(dom0);
+    if ( elf_check_broken(&elf) )
+        printk(" Xen warning: dom0 kernel broken ELF: %s\n",
+               elf_check_broken(&elf));
 
+    iommu_dom0_init(dom0);
     return 0;
+
+out:
+    if ( elf_check_broken(&elf) )
+        printk(" Xen dom0 kernel broken ELF: %s\n",
+               elf_check_broken(&elf));
+
+    return rc;
 }
 
 /*