Xen Security Advisory 95 - input handling vulnerabilities loading guest kernel on ARM

tools: arm: remove code to check for a DTB appended to the kernel

The code to check for an appended DTB was confusing and unnecessary. Since we
know the size of the kernel binary passed to us we should just load the entire
thing into guest RAM (subject to the limits checks). Removing this code avoids
a whole raft of overflow and alignment issues.

We also need to validate the limits of the segment where we intend to load the
kernel to avoid overflow issues.

For ARM32 we control the load address, but we need to validate the size. The
entry point is only relevant within the guest so we don't need to worry about
that.

For ARM64 we need to validate both the load address (which is the same as the
entry point) and the size.

This is XSA-95.

Reported-by: Thomas Leonard <[email protected]>
Signed-off-by: Ian Campbell <[email protected]>
Acked-by: Ian Jackson <[email protected]>

diff --git a/tools/libxc/xc_dom_armzimageloader.c b/tools/libxc/xc_dom_armzimageloader.c
index e6516a1..2b28781 100644
--- a/tools/libxc/xc_dom_armzimageloader.c
+++ b/tools/libxc/xc_dom_armzimageloader.c
@@ -51,7 +51,6 @@ struct minimal_dtb_header {
 static int xc_dom_probe_zimage32_kernel(struct xc_dom_image *dom)
 {
     uint32_t *zimage;
-    uint32_t end;
 
     if ( dom->kernel_blob == NULL )
     {
@@ -73,22 +72,6 @@ static int xc_dom_probe_zimage32_kernel(struct xc_dom_image *dom)
         return -EINVAL;
     }
 
-    end = zimage[ZIMAGE32_END_OFFSET/4];
-
-    /*
-     * Check for an appended DTB.
-     */
-    if ( end + sizeof(struct minimal_dtb_header) < dom->kernel_size ) {
-        struct minimal_dtb_header *dtb_hdr;
-        dtb_hdr = (struct minimal_dtb_header *)(dom->kernel_blob + end);
-        if (ntohl/*be32_to_cpu*/(dtb_hdr->magic) == DTB_MAGIC) {
-            xc_dom_printf(dom->xch, "%s: found an appended DTB", __FUNCTION__);
-            end += ntohl/*be32_to_cpu*/(dtb_hdr->total_size);
-        }
-    }
-
-    dom->kernel_size = end;
-
     return 0;
 }
 
@@ -105,8 +88,20 @@ static int xc_dom_parse_zimage32_kernel(struct xc_dom_image *dom)
 
     /* Do not load kernel at the very first RAM address */
     v_start = rambase + 0x8000;
+
+    if ( dom->kernel_size > UINT64_MAX - v_start )
+    {
+        DOMPRINTF("%s: kernel is too large\n", __FUNCTION__);
+        return -EINVAL;
+    }
+
     v_end = v_start + dom->kernel_size;
 
+    /*
+     * If start is invalid then the guest will start at some invalid
+     * address and crash, but this happens in guest context so doesn't
+     * concern us here.
+     */
     start = zimage[ZIMAGE32_START_OFFSET/4];
 
     if (start == 0)
@@ -187,7 +182,20 @@ static int xc_dom_parse_zimage64_kernel(struct xc_dom_image *dom)
 
     zimage = dom->kernel_blob;
 
+    if ( zimage->text_offset > UINT64_MAX - rambase )
+    {
+        DOMPRINTF("%s: kernel text offset is too large\n", __FUNCTION__);
+        return -EINVAL;
+    }
+
     v_start = rambase + zimage->text_offset;
+
+    if ( dom->kernel_size > UINT64_MAX - v_start )
+    {
+        DOMPRINTF("%s: kernel is too large\n", __FUNCTION__);
+        return -EINVAL;
+    }
+
     v_end = v_start + dom->kernel_size;
 
     dom->kernel_seg.vstart = v_start;