[Xen-changelog] [xen master] tools: arm: remove code to check for a DTB appended to the kernel

[patchbot@xxxxxxx]

commit 2cd6bf3653a2b983c90bf3af42d199c499db4165
Author:     Ian Campbell <ian.campbell@xxxxxxxxxx>
AuthorDate: Wed May 14 15:12:01 2014 +0100
Commit:     Ian Campbell <ian.campbell@xxxxxxxxxx>
CommitDate: Wed May 14 15:12:01 2014 +0100

    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 <talex5@xxxxxxxxx>
    Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
    Acked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxc/xc_dom_armzimageloader.c |   42 ++++++++++++++++++++-------------
 1 files changed, 25 insertions(+), 17 deletions(-)

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;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog