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.

Changes in the Xen 4.2 version of this series:
* No need to fix code relating to ARM.
* No need to fix code relating to superpage support.
* Additionally fix `dom->p2m_host = xc_dom_malloc...' in xc_dom_ia64.c.

Changes in the Xen 4.1 version of this series:
* An additional check is needed in xc_flask.c:xc_flask_access.

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

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index f61308a19647edfcedfc89a617b0283118b9cd71..5adf2d9fca894ed26141450f6a2d9c2a5c3b3974 100644 (file)
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -515,6 +515,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;
 }
@@ -526,6 +528,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';
     }
@@ -599,7 +603,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));
@@ -611,6 +615,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))));
@@ -627,12 +636,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;
 }
 
 /*
@@ -677,6 +688,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_core.c b/tools/libxc/xc_dom_core.c
index a54ddae3d33dd466d9fa28c607e5ae519c74d6a3..3cbf9f791e9c05952e61dda676771fdabd11261d 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 aa6b5f054678f8ce7f344a5e45ed8dc48031ed8d..0fb3629e59407f083c2ac20d9a04efd5451f11fa 100644 (file)
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -329,6 +329,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_ia64.c b/tools/libxc/xc_dom_ia64.c
index 7c0eff15d83f470842e2f88c624d0d137c7de118..076821c19d2885e867a13d8d4f0e350dc91c228d 100644 (file)
--- a/tools/libxc/xc_dom_ia64.c
+++ b/tools/libxc/xc_dom_ia64.c
@@ -188,6 +188,12 @@ int arch_setup_meminit(struct xc_dom_image *dom)
 
     /* setup initial p2m */
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * nbr);
+    if ( dom->p2m_host == NULL )
+    {
+        DOMPRINTF("%s: xc_dom_malloc failed for p2m_host",
+                  __FUNCTION__);
+        return -1;
+    }
     for ( pfn = 0; pfn < nbr; pfn++ )
         dom->p2m_host[pfn] = start + pfn;
 

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 75d6b8336ca4a73349677739262d651fed042ccf..448d9a1e7499fc376c7f4bbe23d0c9681094c663 100644 (file)
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -780,6 +780,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 29af52bac6321b2e48c8914ea19173259739f4c0..0f1e2d16c1ee26f8de4e91e207a41f994bcad5a5 100644 (file)
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -967,6 +967,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);
 

diff --git a/tools/libxc/xc_flask.c b/tools/libxc/xc_flask.c
index 27794a82d59318ceb24caace444c6fa4337b2e8a..78c243c9e045a150b60f6ee8f2bb874572330336 100644 (file)
--- a/tools/libxc/xc_flask.c
+++ b/tools/libxc/xc_flask.c
@@ -284,6 +284,8 @@ int xc_flask_access(xc_interface *xc_handle, const char *scon, const char *tcon,
         MAX_SHORT_DEC_LEN + 1 +
         sizeof(req)*2 + 1;
     buf = malloc(bufLen);
+    if ( buf == NULL )
+        return -ENOMEM;
     snprintf(buf, bufLen, "%s %s %hu %x", scon, tcon, tclass, req);
 
     op.cmd = FLASK_ACCESS;

diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
index 6477ad8607710f5cb81acf2258f31fab26b417fb..fa7bb7c14c88eea7c114ef1134e1ba7760fff7e9 100644 (file)
--- a/tools/libxc/xc_linux_osdep.c
+++ b/tools/libxc/xc_linux_osdep.c
@@ -294,6 +294,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;
@@ -318,6 +320,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 09c8f23ca510441ef80fd9462da5118918a664b4..1bf25d2a1ef40150f690046694ff18c06872827b 100644 (file)
--- a/tools/libxc/xc_private.c
+++ b/tools/libxc/xc_private.c
@@ -742,6 +742,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 570c6d46b5d3cbb70c5199e7cc72dc3c9b85e578..9bc91723769040199eb07b3347a58ac563d1ab60 100644 (file)
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -1608,7 +1608,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