[Xen-changelog] [xen master] libxc: check return values from malloc

[patchbot@xxxxxxx]

commit 3d5a1d4733e55e33521cd5004cab1313e5c5d5ff
Author:     Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
AuthorDate: Fri Jun 14 16:39:38 2013 +0100
Commit:     Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CommitDate: Fri Jun 14 16:39:38 2013 +0100

    libxc: check return values from malloc
    
    A sufficiently malformed input to libxc (such as a malformed input ELF
    or other guest-controlled data) might cause one of libxc's malloc() to
    fail.  In this case we need to make sure we don't dereference or do
    pointer arithmetic on the result.
    
    Search for all occurrences of \b(m|c|re)alloc in libxc, and all
    functions which call them, and add appropriate error checking where
    missing.
    
    This includes the functions xc_dom_malloc*, which now print a message
    when they fail so that callers don't have to do so.
    
    The function xc_cpuid_to_str wasn't provided with a sane return value
    and has a pretty strange API, which now becomes a little stranger.
    There are no in-tree callers.
    
    This is part of the fix to a security issue, XSA-55.
    
    Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    
    v8: Move a check in xc_exchange_page to the previous patch
         (ie, remove it from this patch).
    
    v7: Add a missing check for a call to alloc_str.
        Add arithmetic overflow check in xc_dom_malloc.
        Coding style fix.
    
    v6: Fix a missed call `pfn_err = calloc...' in xc_domain_restore.c.
        Fix a missed call `new_pfn = xc_map_foreign_range...' in
         xc_offline_page.c
    
    v5: This patch is new in this version of the series.
---
 tools/libxc/xc_cpuid_x86.c      |   20 ++++++++++++++++++--
 tools/libxc/xc_dom_arm.c        |    2 ++
 tools/libxc/xc_dom_core.c       |   13 +++++++++++++
 tools/libxc/xc_dom_elfloader.c  |    2 ++
 tools/libxc/xc_dom_x86.c        |    3 +++
 tools/libxc/xc_domain_restore.c |   13 +++++++++++++
 tools/libxc/xc_linux_osdep.c    |    4 ++++
 tools/libxc/xc_private.c        |    2 ++
 tools/libxc/xenctrl.h           |    2 +-
 9 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 17efc0f..fa47787 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -590,6 +590,8 @@ static int xc_cpuid_do_domctl(
 static char *alloc_str(void)
 {
     char *s = malloc(33);
+    if ( s == NULL )
+        return s;
     memset(s, 0, 33);
     return s;
 }
@@ -601,6 +603,8 @@ void xc_cpuid_to_str(const unsigned int *regs, char **strs)
     for ( i = 0; i < 4; i++ )
     {
         strs[i] = alloc_str();
+        if ( strs[i] == NULL )
+            continue;
         for ( j = 0; j < 32; j++ )
             strs[i][j] = !!((regs[i] & (1U << (31 - j)))) ? '1' : '0';
     }
@@ -681,7 +685,7 @@ int xc_cpuid_check(
     const char **config,
     char **config_transformed)
 {
-    int i, j;
+    int i, j, rc;
     unsigned int regs[4];
 
     memset(config_transformed, 0, 4 * sizeof(*config_transformed));
@@ -693,6 +697,11 @@ int xc_cpuid_check(
         if ( config[i] == NULL )
             continue;
         config_transformed[i] = alloc_str();
+        if ( config_transformed[i] == NULL )
+        {
+            rc = -ENOMEM;
+            goto fail_rc;
+        }
         for ( j = 0; j < 32; j++ )
         {
             unsigned char val = !!((regs[i] & (1U << (31 - j))));
@@ -709,12 +718,14 @@ int xc_cpuid_check(
     return 0;
 
  fail:
+    rc = -EPERM;
+ fail_rc:
     for ( i = 0; i < 4; i++ )
     {
         free(config_transformed[i]);
         config_transformed[i] = NULL;
     }
-    return -EPERM;
+    return rc;
 }
 
 /*
@@ -759,6 +770,11 @@ int xc_cpuid_set(
         }
         
         config_transformed[i] = alloc_str();
+        if ( config_transformed[i] == NULL )
+        {
+            rc = -ENOMEM;
+            goto fail;
+        }
 
         for ( j = 0; j < 32; j++ )
         {
diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index aaf35ca..df59ffb 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -170,6 +170,8 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     dom->shadow_enabled = 1;
 
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
+    if ( dom->p2m_host == NULL )
+        return -EINVAL;
 
     /* setup initial p2m */
     for ( pfn = 0; pfn < dom->total_pages; pfn++ )
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 21a8e0d..1a14d3c 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -120,9 +120,17 @@ void *xc_dom_malloc(struct xc_dom_image *dom, size_t size)
 {
     struct xc_dom_mem *block;
 
+    if ( size > SIZE_MAX - sizeof(*block) )
+    {
+        DOMPRINTF("%s: unreasonable allocation size", __FUNCTION__);
+        return NULL;
+    }
     block = malloc(sizeof(*block) + size);
     if ( block == NULL )
+    {
+        DOMPRINTF("%s: allocation failed", __FUNCTION__);
         return NULL;
+    }
     memset(block, 0, sizeof(*block) + size);
     block->next = dom->memblocks;
     dom->memblocks = block;
@@ -138,7 +146,10 @@ void *xc_dom_malloc_page_aligned(struct xc_dom_image *dom, 
size_t size)
 
     block = malloc(sizeof(*block));
     if ( block == NULL )
+    {
+        DOMPRINTF("%s: allocation failed", __FUNCTION__);
         return NULL;
+    }
     memset(block, 0, sizeof(*block));
     block->mmap_len = size;
     block->mmap_ptr = mmap(NULL, block->mmap_len,
@@ -146,6 +157,7 @@ void *xc_dom_malloc_page_aligned(struct xc_dom_image *dom, 
size_t size)
                            -1, 0);
     if ( block->mmap_ptr == MAP_FAILED )
     {
+        DOMPRINTF("%s: mmap failed", __FUNCTION__);
         free(block);
         return NULL;
     }
@@ -202,6 +214,7 @@ void *xc_dom_malloc_filemap(struct xc_dom_image *dom,
         close(fd);
     if ( block != NULL )
         free(block);
+    DOMPRINTF("%s: failed (on file `%s')", __FUNCTION__, filename);
     return NULL;
 }
 
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 8d0a09f..9843b1f 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -327,6 +327,8 @@ static elf_errorstatus xc_dom_parse_elf_kernel(struct 
xc_dom_image *dom)
         return rc;
 
     elf = xc_dom_malloc(dom, sizeof(*elf));
+    if ( elf == NULL )
+        return -1;
     dom->private_loader = elf;
     rc = elf_init(elf, dom->kernel_blob, dom->kernel_size);
     xc_elf_set_logfile(dom->xch, elf, 1);
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 8b6191d..126c0f8 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -760,6 +760,9 @@ int arch_setup_meminit(struct xc_dom_image *dom)
     }
 
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->total_pages);
+    if ( dom->p2m_host == NULL )
+        return -EINVAL;
+
     if ( dom->superpages )
     {
         int count = dom->total_pages >> SUPERPAGE_PFN_SHIFT;
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index c7835ff..f53ff88 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1243,6 +1243,11 @@ static int apply_batch(xc_interface *xch, uint32_t dom, 
struct restore_ctx *ctx,
 
     /* Map relevant mfns */
     pfn_err = calloc(j, sizeof(*pfn_err));
+    if ( pfn_err == NULL )
+    {
+        PERROR("allocation for pfn_err failed");
+        return -1;
+    }
     region_base = xc_map_foreign_bulk(
         xch, dom, PROT_WRITE, region_mfn, pfn_err, j);
 
@@ -1532,8 +1537,16 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
uint32_t dom,
     region_mfn = malloc(ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), 
PAGE_SHIFT));
     ctx->p2m_batch = malloc(ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), 
PAGE_SHIFT));
     if (!ctx->hvm && ctx->superpages)
+    {
         ctx->p2m_saved_batch =
             malloc(ROUNDUP(MAX_BATCH_SIZE * sizeof(xen_pfn_t), PAGE_SHIFT));
+        if ( ctx->p2m_saved_batch == NULL )
+        {
+            ERROR("saved batch memory alloc failed");
+            errno = ENOMEM;
+            goto out;
+        }
+    }
 
     if ( (ctx->p2m == NULL) || (pfn_type == NULL) ||
          (region_mfn == NULL) || (ctx->p2m_batch == NULL) )
diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
index 36832b6..73860a2 100644
--- a/tools/libxc/xc_linux_osdep.c
+++ b/tools/libxc/xc_linux_osdep.c
@@ -378,6 +378,8 @@ static void *linux_privcmd_map_foreign_range(xc_interface 
*xch, xc_osdep_handle
 
     num = (size + XC_PAGE_SIZE - 1) >> XC_PAGE_SHIFT;
     arr = calloc(num, sizeof(xen_pfn_t));
+    if ( arr == NULL )
+        return NULL;
 
     for ( i = 0; i < num; i++ )
         arr[i] = mfn + i;
@@ -402,6 +404,8 @@ static void *linux_privcmd_map_foreign_ranges(xc_interface 
*xch, xc_osdep_handle
     num_per_entry = chunksize >> XC_PAGE_SHIFT;
     num = num_per_entry * nentries;
     arr = calloc(num, sizeof(xen_pfn_t));
+    if ( arr == NULL )
+        return NULL;
 
     for ( i = 0; i < nentries; i++ )
         for ( j = 0; j < num_per_entry; j++ )
diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
index e891cc8..acaf9e0 100644
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -771,6 +771,8 @@ const char *xc_strerror(xc_interface *xch, int errcode)
         errbuf = pthread_getspecific(errbuf_pkey);
         if (errbuf == NULL) {
             errbuf = malloc(XS_BUFSIZE);
+            if ( errbuf == NULL )
+                return "(failed to allocate errbuf)";
             pthread_setspecific(errbuf_pkey, errbuf);
         }
 
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 40ee8fc..5697765 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1827,7 +1827,7 @@ int xc_cpuid_set(xc_interface *xch,
 int xc_cpuid_apply_policy(xc_interface *xch,
                           domid_t domid);
 void xc_cpuid_to_str(const unsigned int *regs,
-                     char **strs);
+                     char **strs); /* some strs[] may be NULL if ENOMEM */
 int xc_mca_op(xc_interface *xch, struct xen_mc *mc);
 #endif
 
--
generated by git-patchbot for /home/xen/git/xen.git#master

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