From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: andrew.cooper3@citrix.com, mattjd@gmail.com,
Ian Jackson <ian.jackson@eu.citrix.com>,
security@xen.org
Subject: [PATCH 18/21] DRAFT libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
Date: Wed, 12 Jun 2013 17:00:37 +0100 [thread overview]
Message-ID: <1371052840-10475-19-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1371052840-10475-1-git-send-email-ian.jackson@eu.citrix.com>
*** DRAFT ONLY ***
This is a direct backport of the 4.2 patch. I have not rerun the
search yet, and need to do so.
The return values from xc_dom_*_to_ptr and xc_map_foreign_range are
sometimes dereferenced, or subjected to pointer arithmetic, without
checking whether the relevant function failed and returned NULL.
Add an appropriate error check at every call site.
Changes in the 4.2 backport of this series:
* Fix tools/libxc/xc_dom_x86.c:setup_pgtables_x86_32.
* Fix tools/libxc/xc_dom_ia64.c:start_info_ia64.
* Fix tools/libxc/ia64/xc_ia64_dom_fwloader.c:xc_dom_load_fw_kernel.
Conflicts in the 4.1 backport of this series:
* xc_dom_load_elf_kernel has less error handling in 4.1.
* the VM generation ID code is not in 4.1.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxc/ia64/xc_ia64_dom_fwloader.c | 2 +
tools/libxc/xc_dom_binloader.c | 6 +++
tools/libxc/xc_dom_core.c | 6 +++
tools/libxc/xc_dom_elfloader.c | 12 +++++++
tools/libxc/xc_dom_ia64.c | 6 +++
tools/libxc/xc_dom_x86.c | 55 +++++++++++++++++++++++++++++++
tools/libxc/xc_domain_restore.c | 21 ++++++++++++
7 files changed, 108 insertions(+), 0 deletions(-)
diff --git a/tools/libxc/ia64/xc_ia64_dom_fwloader.c b/tools/libxc/ia64/xc_ia64_dom_fwloader.c
index cdf3333..dbd3349 100644
--- a/tools/libxc/ia64/xc_ia64_dom_fwloader.c
+++ b/tools/libxc/ia64/xc_ia64_dom_fwloader.c
@@ -60,6 +60,8 @@ static int xc_dom_load_fw_kernel(struct xc_dom_image *dom)
unsigned long i;
dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart);
+ if ( dest == NULL )
+ return -1;
memcpy(dest, dom->kernel_blob, FW_SIZE);
/* Synchronize cache. */
diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index bb904b3..e3ecdaf 100644
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -277,6 +277,12 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
DOMPRINTF(" bss_size: 0x%" PRIx32 "", bss_size);
dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart, &dest_size);
+ if ( dest == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart)"
+ " => NULL", __FUNCTION__);
+ return -EINVAL;
+ }
if ( dest_size < text_size ||
dest_size - text_size < bss_size )
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index 8913e41..a54ddae 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -868,6 +868,12 @@ int xc_dom_build_image(struct xc_dom_image *dom)
ramdisklen) != 0 )
goto err;
ramdiskmap = xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg);
+ if ( ramdiskmap == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, &dom->ramdisk_seg) => NULL",
+ __FUNCTION__);
+ goto err;
+ }
if ( unziplen )
{
if ( xc_dom_do_gunzip(dom->xch,
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 8f2497d..a0d1980 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -139,6 +139,12 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom,
return 0;
size = dom->kernel_seg.vend - dom->bsd_symtab_start;
hdr_ptr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start, &allow_size);
+ if ( hdr_ptr == NULL )
+ {
+ DOMPRINTF("%s/load: xc_dom_vaddr_to_ptr(dom,dom->bsd_symtab_start"
+ " => NULL", __FUNCTION__);
+ return -1;
+ }
elf->caller_xdest_base = hdr_ptr;
elf->caller_xdest_size = allow_size;
hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
@@ -375,6 +381,12 @@ static elf_errorstatus xc_dom_load_elf_kernel(struct xc_dom_image *dom)
xen_pfn_t pages;
elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg, &pages);
+ if ( elf->dest_base == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_vaddr_to_ptr(dom,dom->kernel_seg)"
+ " => NULL", __FUNCTION__);
+ return -1;
+ }
elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom);
elf_load_binary(elf);
if ( dom->parms.bsd_symtab )
diff --git a/tools/libxc/xc_dom_ia64.c b/tools/libxc/xc_dom_ia64.c
index dcd1523..7c0eff1 100644
--- a/tools/libxc/xc_dom_ia64.c
+++ b/tools/libxc/xc_dom_ia64.c
@@ -60,6 +60,12 @@ int start_info_ia64(struct xc_dom_image *dom)
DOMPRINTF_CALLED(dom->xch);
+ if ( start_info == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_pfn_to_ptr failed on start_info", __FUNCTION__);
+ return -1; /* our caller throws away our return value :-/ */
+ }
+
memset(start_info, 0, sizeof(*start_info));
sprintf(start_info->magic, dom->guest_type);
start_info->flags = dom->flags;
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 0cf1687..75d6b83 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -144,6 +144,9 @@ static int setup_pgtables_x86_32(struct xc_dom_image *dom)
xen_vaddr_t addr;
xen_pfn_t pgpfn;
+ if ( l2tab == NULL )
+ goto pfn_error;
+
for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
addr += PAGE_SIZE_X86 )
{
@@ -151,6 +154,8 @@ static int setup_pgtables_x86_32(struct xc_dom_image *dom)
{
/* get L1 tab, make L2 entry */
l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+ if ( l1tab == NULL )
+ goto pfn_error;
l2off = l2_table_offset_i386(addr);
l2tab[l2off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -169,6 +174,11 @@ static int setup_pgtables_x86_32(struct xc_dom_image *dom)
l1tab = NULL;
}
return 0;
+
+pfn_error:
+ xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+ "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+ return -EINVAL;
}
/*
@@ -219,6 +229,12 @@ static xen_pfn_t move_l3_below_4G(struct xc_dom_image *dom,
goto out;
l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+ if ( l3tab == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_pfn_to_ptr(dom, l3pfn, 1) => NULL",
+ __FUNCTION__);
+ return l3mfn; /* our one call site will call xc_dom_panic and fail */
+ }
memset(l3tab, 0, XC_DOM_PAGE_SIZE(dom));
DOMPRINTF("%s: successfully relocated L3 below 4G. "
@@ -262,6 +278,8 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
}
l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+ if ( l3tab == NULL )
+ goto pfn_error;
for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
addr += PAGE_SIZE_X86 )
@@ -270,6 +288,8 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
{
/* get L2 tab, make L3 entry */
l2tab = xc_dom_pfn_to_ptr(dom, l2pfn, 1);
+ if ( l2tab == NULL )
+ goto pfn_error;
l3off = l3_table_offset_pae(addr);
l3tab[l3off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
@@ -280,6 +300,8 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
{
/* get L1 tab, make L2 entry */
l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+ if ( l1tab == NULL )
+ goto pfn_error;
l2off = l2_table_offset_pae(addr);
l2tab[l2off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -306,6 +328,11 @@ static int setup_pgtables_x86_32_pae(struct xc_dom_image *dom)
l3tab[3] = pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
}
return 0;
+
+pfn_error:
+ xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+ "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+ return -EINVAL;
}
#undef L1_PROT
@@ -344,6 +371,9 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
uint64_t addr;
xen_pfn_t pgpfn;
+ if ( l4tab == NULL )
+ goto pfn_error;
+
for ( addr = dom->parms.virt_base; addr < dom->virt_pgtab_end;
addr += PAGE_SIZE_X86 )
{
@@ -351,6 +381,8 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
{
/* get L3 tab, make L4 entry */
l3tab = xc_dom_pfn_to_ptr(dom, l3pfn, 1);
+ if ( l3tab == NULL )
+ goto pfn_error;
l4off = l4_table_offset_x86_64(addr);
l4tab[l4off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l3pfn)) | L4_PROT;
@@ -361,6 +393,8 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
{
/* get L2 tab, make L3 entry */
l2tab = xc_dom_pfn_to_ptr(dom, l2pfn, 1);
+ if ( l2tab == NULL )
+ goto pfn_error;
l3off = l3_table_offset_x86_64(addr);
l3tab[l3off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l2pfn)) | L3_PROT;
@@ -373,6 +407,8 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
{
/* get L1 tab, make L2 entry */
l1tab = xc_dom_pfn_to_ptr(dom, l1pfn, 1);
+ if ( l1tab == NULL )
+ goto pfn_error;
l2off = l2_table_offset_x86_64(addr);
l2tab[l2off] =
pfn_to_paddr(xc_dom_p2m_guest(dom, l1pfn)) | L2_PROT;
@@ -393,6 +429,11 @@ static int setup_pgtables_x86_64(struct xc_dom_image *dom)
l1tab = NULL;
}
return 0;
+
+pfn_error:
+ xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
+ "%s: xc_dom_pfn_to_ptr failed", __FUNCTION__);
+ return -EINVAL;
}
#undef L1_PROT
@@ -410,6 +451,8 @@ static int alloc_magic_pages(struct xc_dom_image *dom)
if ( xc_dom_alloc_segment(dom, &dom->p2m_seg, "phys2mach", 0, p2m_size) )
return -1;
dom->p2m_guest = xc_dom_seg_to_ptr(dom, &dom->p2m_seg);
+ if ( dom->p2m_guest == NULL )
+ return -1;
/* allocate special pages */
dom->start_info_pfn = xc_dom_alloc_page(dom, "start info");
@@ -434,6 +477,12 @@ static int start_info_x86_32(struct xc_dom_image *dom)
DOMPRINTF_CALLED(dom->xch);
+ if ( start_info == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_pfn_to_ptr failed on start_info", __FUNCTION__);
+ return -1; /* our caller throws away our return value :-/ */
+ }
+
memset(start_info, 0, sizeof(*start_info));
strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
start_info->magic[sizeof(start_info->magic) - 1] = '\0';
@@ -474,6 +523,12 @@ static int start_info_x86_64(struct xc_dom_image *dom)
DOMPRINTF_CALLED(dom->xch);
+ if ( start_info == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_pfn_to_ptr failed on start_info", __FUNCTION__);
+ return -1; /* our caller throws away our return value :-/ */
+ }
+
memset(start_info, 0, sizeof(*start_info));
strncpy(start_info->magic, dom->guest_type, sizeof(start_info->magic));
start_info->magic[sizeof(start_info->magic) - 1] = '\0';
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index 9564480..29af52b 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1434,6 +1434,11 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
l3tab = (uint64_t *)
xc_map_foreign_range(xch, dom, PAGE_SIZE,
PROT_READ, ctx->p2m[i]);
+ if ( l3tab == NULL )
+ {
+ PERROR("xc_map_foreign_range failed (for l3tab)");
+ goto out;
+ }
for ( j = 0; j < 4; j++ )
l3ptes[j] = l3tab[j];
@@ -1460,6 +1465,11 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
l3tab = (uint64_t *)
xc_map_foreign_range(xch, dom, PAGE_SIZE,
PROT_READ | PROT_WRITE, ctx->p2m[i]);
+ if ( l3tab == NULL )
+ {
+ PERROR("xc_map_foreign_range failed (for l3tab, 2nd)");
+ goto out;
+ }
for ( j = 0; j < 4; j++ )
l3tab[j] = l3ptes[j];
@@ -1630,6 +1640,12 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
SET_FIELD(ctxt, user_regs.edx, mfn);
start_info = xc_map_foreign_range(
xch, dom, PAGE_SIZE, PROT_READ | PROT_WRITE, mfn);
+ if ( start_info == NULL )
+ {
+ PERROR("xc_map_foreign_range failed (for start_info)");
+ goto out;
+ }
+
SET_FIELD(start_info, nr_pages, dinfo->p2m_size);
SET_FIELD(start_info, shared_info, shared_info_frame<<PAGE_SHIFT);
SET_FIELD(start_info, flags, 0);
@@ -1765,6 +1781,11 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
/* Restore contents of shared-info page. No checking needed. */
new_shared_info = xc_map_foreign_range(
xch, dom, PAGE_SIZE, PROT_WRITE, shared_info_frame);
+ if ( new_shared_info == NULL )
+ {
+ PERROR("xc_map_foreign_range failed (for new_shared_info)");
+ goto out;
+ }
/* restore saved vcpu_info and arch specific info */
MEMCPY_FIELD(new_shared_info, old_shared_info, vcpu_info);
--
1.7.2.5
next prev parent reply other threads:[~2013-06-12 16:00 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-12 16:00 [PATCH RFC v7 00/22] XSA55 libelf fixes for Xen 4.1 Ian Jackson
2013-06-12 16:00 ` [PATCH 01/21] libelf: abolish libelf-relocate.c Ian Jackson
2013-06-12 16:00 ` [PATCH 02/21] libxc: introduce xc_dom_seg_to_ptr_pages Ian Jackson
2013-06-12 16:00 ` [PATCH 03/21] libxc: Fix range checking in xc_dom_pfn_to_ptr etc Ian Jackson
2013-06-12 16:00 ` [PATCH 04/21] libelf: abolish elf_sval and elf_access_signed Ian Jackson
2013-06-12 16:00 ` [PATCH 05/21] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised Ian Jackson
2013-06-12 16:00 ` [PATCH 06/21] libelf: introduce macros for memory access and pointer handling Ian Jackson
2013-06-12 16:00 ` [PATCH 07/21] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Ian Jackson
2013-06-12 16:00 ` [PATCH 08/21] libelf: check nul-terminated strings properly Ian Jackson
2013-06-12 16:00 ` [PATCH 09/21] libelf: check all pointer accesses Ian Jackson
2013-06-12 16:00 ` [PATCH 10/21] libelf: Check pointer references in elf_is_elfbinary Ian Jackson
2013-06-12 16:00 ` [PATCH 11/21] libelf: Make all callers call elf_check_broken Ian Jackson
2013-06-12 16:00 ` [PATCH 12/21] libelf: use C99 bool for booleans Ian Jackson
2013-06-12 16:00 ` [PATCH 13/21] libelf: use only unsigned integers Ian Jackson
2013-06-12 16:29 ` George Dunlap
2013-06-12 17:48 ` AL13N
2013-06-13 9:51 ` George Dunlap
2013-06-13 13:33 ` Ian Jackson
2013-06-12 16:00 ` [PATCH 14/21] libxc: Introduce xc_bitops.h Ian Jackson
2013-06-12 16:00 ` [PATCH 15/21] libelf: check loops for running away Ian Jackson
2013-06-13 16:47 ` George Dunlap
2013-06-13 17:34 ` Ian Jackson
2013-06-13 17:54 ` Ian Jackson
2013-06-12 16:00 ` [PATCH 16/21] libelf: abolish obsolete macros Ian Jackson
2013-06-12 16:00 ` [PATCH 17/21] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-12 16:00 ` Ian Jackson [this message]
2013-06-13 15:54 ` [PATCH 18/21] DRAFT libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range Ian Jackson
2013-06-12 16:00 ` [PATCH 19/21] DRAFT libxc: check return values from malloc Ian Jackson
2013-06-13 15:53 ` Ian Jackson
2013-06-12 16:00 ` [PATCH 20/21] libxc: range checks in xc_dom_p2m_host and _guest Ian Jackson
2013-06-12 16:00 ` [PATCH 21/21] libxc: check blob size before proceeding in xc_dom_check_gzip Ian Jackson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1371052840-10475-19-git-send-email-ian.jackson@eu.citrix.com \
--to=ian.jackson@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=mattjd@gmail.com \
--cc=security@xen.org \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).