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@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
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.

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 17efc0f841317f8014931e395b91c271b35b44a9..fa47787d3e11ae2161cf67c21ce05eb70f858051 100644 (file)
--- 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 aaf35ca70dc68a475e61accaadd7912a941da44c..df59ffb84c96627efe5d5d71ed596e0ff451fecf 100644 (file)
--- 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 21a8e0deb62a0dc493c8bdcf6fd3e49c35cf7f8c..1a14d3c5ce41a1c2381b83f0664e82109e6493ec 100644 (file)
--- 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 8d0a09f752e33be8a71f4206d0ab325acaeef312..9843b1f723410b8e829be0d0d543baa44a920d6f 100644 (file)
--- 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 8b6191d25bbdaec744776f58a01ab0e61aa120ed..126c0f8eeb5c6fa698f1be28091086c1000a8238 100644 (file)
--- 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 c7835ff6a918e456ec36a085e48f954e5b7b1a93..f53ff8823789367a784ea66a10993d82ce60eb85 100644 (file)
--- 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 36832b615339aea48abab5456e8c6bc21e6c01c2..73860a269988c4b2f93508a197851bc12c0b9d76 100644 (file)
--- 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 e891cc804762541e57a4717d47a6b0f167218015..acaf9e0e1e239622a6ca056723e41bed7807b7a1 100644 (file)
--- 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 40ee8fce2a584149389e11fd888f62e00766a7f5..5697765a17cfa2ce3259f6637843e434e8ef78e6 100644 (file)
--- 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