From a2986a7959919bc748784bb75970bfbd42697d3b Mon Sep 17 00:00:00 2001

From: Ian Jackson <ian.jackson@eu.citrix.com>

Date: Fri, 14 Jun 2013 16:45:41 +0100

Subject: [PATCH 19/21] 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>

---

 tools/libxc/xc_cpuid_x86.c      |   20 ++++++++++++++++++--

 tools/libxc/xc_dom_core.c       |   13 +++++++++++++

 tools/libxc/xc_dom_elfloader.c  |    2 ++

 tools/libxc/xc_dom_ia64.c       |    6 ++++++

 tools/libxc/xc_dom_x86.c        |    3 +++

 tools/libxc/xc_domain_restore.c |    5 +++++

 tools/libxc/xc_flask.c          |    2 ++

 tools/libxc/xc_linux_osdep.c    |    4 ++++

 tools/libxc/xc_private.c        |    2 ++

 tools/libxc/xenctrl.h           |    2 +-

 10 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c

index f61308a..5adf2d9 100644

--- 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 a54ddae..3cbf9f7 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 aa6b5f0..0fb3629 100644

--- 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 7c0eff1..076821c 100644

--- 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 75d6b83..448d9a1 100644

--- 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 29af52b..0f1e2d1 100644

--- 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 27794a8..78c243c 100644

--- 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 6477ad8..fa7bb7c 100644

--- 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 09c8f23..1bf25d2 100644

--- 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 570c6d4..9bc9172 100644

--- 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

-- 

1.7.2.5