|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 20/22] libxc: check return values from malloc
On 07/06/13 19:27, Ian Jackson wrote:
> 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>
>
> 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 | 15 +++++++++++++--
> tools/libxc/xc_dom_arm.c | 2 ++
> tools/libxc/xc_dom_core.c | 9 ++++++++-
> 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_offline_page.c | 5 +++++
> tools/libxc/xc_private.c | 2 ++
> tools/libxc/xenctrl.h | 2 +-
> 10 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 17efc0f..7480a86 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;
> }
>
> /*
> 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..2a9c5a2 100644
> --- a/tools/libxc/xc_dom_core.c
> +++ b/tools/libxc/xc_dom_core.c
> @@ -122,7 +122,10 @@ void *xc_dom_malloc(struct xc_dom_image *dom, size_t
> size)
>
> 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;
> @@ -137,8 +140,10 @@ void *xc_dom_malloc_page_aligned(struct xc_dom_image
> *dom, size_t size)
> struct xc_dom_mem *block;
>
> block = malloc(sizeof(*block));
> - if ( block == NULL )
> + if ( block == NULL ) {
Only a small nit - Mix of coding styles. Xen coding style would mean
this brace should be on a new line.
~Andrew
> + 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 +151,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 +208,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 ac9bb3b..ae427ef 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_offline_page.c b/tools/libxc/xc_offline_page.c
> index 089a361..5aad79d 100644
> --- a/tools/libxc/xc_offline_page.c
> +++ b/tools/libxc/xc_offline_page.c
> @@ -714,6 +714,11 @@ int xc_exchange_page(xc_interface *xch, int domid,
> xen_pfn_t mfn)
>
> new_p = xc_map_foreign_range(xch, domid, PAGE_SIZE,
> PROT_READ|PROT_WRITE, new_mfn);
> + if ( new_p == NULL )
> + {
> + ERROR("failed to map foreign range for new_p");
> + goto failed;
> + }
> memcpy(new_p, backup, PAGE_SIZE);
> munmap(new_p, PAGE_SIZE);
> mops.arg1.mfn = new_mfn;
> 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 c024af4..f6f4ceb 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1829,7 +1829,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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |