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".

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

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

v5: Fix two whitespace errors.

v3.1:
    Add error check to xc_dom_parse_elf_kernel.
    Move check in xc_hvm_build_x86.c:setup_guest to right place.

v2 was Acked-by: Ian Campbell <ian.campbell@citrix.com>
v2 was Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

v2: Style fixes.

2 files changed, 24 insertions, 4 deletions

diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index f14b053186..a0d39b31ae 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -274,6 +274,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__);
@@ -310,19 +317,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 */
@@ -336,7 +347,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 eff55a48ac..8bb0178dc5 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -524,6 +524,9 @@ static int setup_guest(xc_interface *xch,
  error_out:
     rc = -1;
  out:
+    if ( elf_check_broken(&elf) )
+        ERROR("HVM ELF broken: %s", elf_check_broken(&elf));
+
     /* ensure no unclaimed pages are left unused */
     xc_domain_claim_pages(xch, dom, 0 /* cancels the claim */);