371 lines
12 KiB
Diff
371 lines
12 KiB
Diff
From d0790bdad7496e720416b2d4a04563c4c27e7b95 Mon Sep 17 00:00:00 2001
|
|
From: Ian Jackson <ian.jackson@eu.citrix.com>
|
|
Date: Fri, 14 Jun 2013 16:43:17 +0100
|
|
Subject: [PATCH 12/23] libelf: Check pointer references in elf_is_elfbinary
|
|
|
|
elf_is_elfbinary didn't take a length parameter and could potentially
|
|
access out of range when provided with a very short image.
|
|
|
|
We only need to check the size is enough for the actual dereference in
|
|
elf_is_elfbinary; callers are just using it to check the magic number
|
|
and do their own checks (usually via the new elf_ptrval system) before
|
|
dereferencing other parts of the header.
|
|
|
|
This is part of the fix to a security issue, XSA-55.
|
|
|
|
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
|
|
Acked-by: Ian Campbell <ian.campbell@citrix.com>
|
|
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
|
|
---
|
|
tools/libxc/xc_dom_elfloader.c | 2 +-
|
|
xen/arch/x86/bzimage.c | 4 ++--
|
|
xen/common/libelf/libelf-loader.c | 2 +-
|
|
xen/common/libelf/libelf-tools.c | 9 ++++++---
|
|
xen/include/xen/libelf.h | 4 +++-
|
|
5 files changed, 13 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
|
|
index b82a08c..ea45886 100644
|
|
--- a/tools/libxc/xc_dom_elfloader.c
|
|
+++ b/tools/libxc/xc_dom_elfloader.c
|
|
@@ -95,7 +95,7 @@ static int check_elf_kernel(struct xc_dom_image *dom, int verbose)
|
|
return -EINVAL;
|
|
}
|
|
|
|
- if ( !elf_is_elfbinary(dom->kernel_blob) )
|
|
+ if ( !elf_is_elfbinary(dom->kernel_blob, dom->kernel_size) )
|
|
{
|
|
if ( verbose )
|
|
xc_dom_panic(dom->xch,
|
|
diff --git a/xen/arch/x86/bzimage.c b/xen/arch/x86/bzimage.c
|
|
index 5adc223..3600dca 100644
|
|
--- a/xen/arch/x86/bzimage.c
|
|
+++ b/xen/arch/x86/bzimage.c
|
|
@@ -220,7 +220,7 @@ unsigned long __init bzimage_headroom(char *image_start,
|
|
image_length = hdr->payload_length;
|
|
}
|
|
|
|
- if ( elf_is_elfbinary(image_start) )
|
|
+ if ( elf_is_elfbinary(image_start, image_length) )
|
|
return 0;
|
|
|
|
orig_image_len = image_length;
|
|
@@ -251,7 +251,7 @@ int __init bzimage_parse(char *image_base, char **image_start, unsigned long *im
|
|
*image_len = hdr->payload_length;
|
|
}
|
|
|
|
- if ( elf_is_elfbinary(*image_start) )
|
|
+ if ( elf_is_elfbinary(*image_start, *image_len) )
|
|
return 0;
|
|
|
|
BUG_ON(!(image_base < *image_start));
|
|
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
|
|
index a3310e7..f8be635 100644
|
|
--- a/xen/common/libelf/libelf-loader.c
|
|
+++ b/xen/common/libelf/libelf-loader.c
|
|
@@ -29,7 +29,7 @@ int elf_init(struct elf_binary *elf, const char *image_input, size_t size)
|
|
ELF_HANDLE_DECL(elf_shdr) shdr;
|
|
uint64_t i, count, section, offset;
|
|
|
|
- if ( !elf_is_elfbinary(image_input) )
|
|
+ if ( !elf_is_elfbinary(image_input, size) )
|
|
{
|
|
elf_err(elf, "%s: not an ELF binary\n", __FUNCTION__);
|
|
return -1;
|
|
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
|
|
index 46ca553..744027e 100644
|
|
--- a/xen/common/libelf/libelf-tools.c
|
|
+++ b/xen/common/libelf/libelf-tools.c
|
|
@@ -332,11 +332,14 @@ ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(
|
|
|
|
/* ------------------------------------------------------------------------ */
|
|
|
|
-int elf_is_elfbinary(const void *image)
|
|
+int elf_is_elfbinary(const void *image_start, size_t image_size)
|
|
{
|
|
- const Elf32_Ehdr *ehdr = image;
|
|
+ const Elf32_Ehdr *ehdr = image_start;
|
|
|
|
- return IS_ELF(*ehdr); /* fixme unchecked */
|
|
+ if ( image_size < sizeof(*ehdr) )
|
|
+ return 0;
|
|
+
|
|
+ return IS_ELF(*ehdr);
|
|
}
|
|
|
|
int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr)
|
|
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
|
|
index ddc3ed7..ac93858 100644
|
|
--- a/xen/include/xen/libelf.h
|
|
+++ b/xen/include/xen/libelf.h
|
|
@@ -350,7 +350,9 @@ uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
|
|
unsigned int unitsz, unsigned int idx);
|
|
ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note);
|
|
|
|
-int elf_is_elfbinary(const void *image);
|
|
+/* (Only) checks that the image has the right magic number. */
|
|
+int elf_is_elfbinary(const void *image_start, size_t image_size);
|
|
+
|
|
int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr);
|
|
|
|
/* ------------------------------------------------------------------------ */
|
|
--
|
|
1.7.2.5
|
|
#From a965b8f80388603d439ae2b8ee7b9b018a079f90 Mon Sep 17 00:00:00 2001
|
|
#From: Ian Jackson <ian.jackson@eu.citrix.com>
|
|
#Date: Fri, 14 Jun 2013 16:43:17 +0100
|
|
#Subject: [PATCH 13/23] libelf: Make all callers call elf_check_broken
|
|
#
|
|
#This arranges that if the new pointer reference error checking
|
|
#tripped, we actually get a message about it. In this patch these
|
|
#messages do not change the actual return values from the various
|
|
#functions: so pointer reference errors do not prevent loading. This
|
|
#is for fear that some existing kernels might cause the code to make
|
|
#these wild references, which would then break, which is not a good
|
|
#thing in a security patch.
|
|
#
|
|
#In xen/arch/x86/domain_build.c we have to introduce an "out" label and
|
|
#change all of the "return rc" beyond the relevant point into "goto
|
|
#out".
|
|
#
|
|
#Difference in the 4.2 series, compared to unstable:
|
|
#
|
|
#* tools/libxc/xc_hvm_build_x86.c:setup_guest and
|
|
# xen/arch/arm/kernel.c:kernel_try_elf_prepare have different
|
|
# error handling in 4.2 to unstable; patch adjusted accordingly.
|
|
#
|
|
#This is part of the fix to a security issue, XSA-55.
|
|
#
|
|
#Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
|
|
#
|
|
#xen-unstable version Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
|
|
#---
|
|
# tools/libxc/xc_dom_elfloader.c | 25 +++++++++++++++++++++----
|
|
# tools/libxc/xc_hvm_build_x86.c | 5 +++++
|
|
# tools/xcutils/readnotes.c | 3 +++
|
|
# xen/arch/arm/kernel.c | 15 ++++++++++++++-
|
|
# xen/arch/x86/domain_build.c | 28 +++++++++++++++++++++-------
|
|
# 5 files changed, 64 insertions(+), 12 deletions(-)
|
|
#
|
|
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
|
|
index ea45886..4fb4da2 100644
|
|
--- a/tools/libxc/xc_dom_elfloader.c
|
|
+++ b/tools/libxc/xc_dom_elfloader.c
|
|
@@ -276,6 +276,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
|
|
elf_store_field(elf, shdr, e32.sh_name, 0);
|
|
}
|
|
|
|
+ if ( elf_check_broken(&syms) )
|
|
+ DOMPRINTF("%s: symbols ELF broken: %s", __FUNCTION__,
|
|
+ elf_check_broken(&syms));
|
|
+ if ( elf_check_broken(elf) )
|
|
+ DOMPRINTF("%s: ELF broken: %s", __FUNCTION__,
|
|
+ elf_check_broken(elf));
|
|
+
|
|
if ( tables == 0 )
|
|
{
|
|
DOMPRINTF("%s: no symbol table present", __FUNCTION__);
|
|
@@ -312,19 +319,23 @@ static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
|
|
{
|
|
xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: ELF image"
|
|
" has no shstrtab", __FUNCTION__);
|
|
- return -EINVAL;
|
|
+ rc = -EINVAL;
|
|
+ goto out;
|
|
}
|
|
|
|
/* parse binary and get xen meta info */
|
|
elf_parse_binary(elf);
|
|
if ( (rc = elf_xen_parse(elf, &dom->parms)) != 0 )
|
|
- return rc;
|
|
+ {
|
|
+ goto out;
|
|
+ }
|
|
|
|
if ( elf_xen_feature_get(XENFEAT_dom0, dom->parms.f_required) )
|
|
{
|
|
xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: Kernel does not"
|
|
" support unprivileged (DomU) operation", __FUNCTION__);
|
|
- return -EINVAL;
|
|
+ rc = -EINVAL;
|
|
+ goto out;
|
|
}
|
|
|
|
/* find kernel segment */
|
|
@@ -338,7 +349,13 @@ static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
|
|
DOMPRINTF("%s: %s: 0x%" PRIx64 " -> 0x%" PRIx64 "",
|
|
__FUNCTION__, dom->guest_type,
|
|
dom->kernel_seg.vstart, dom->kernel_seg.vend);
|
|
- return 0;
|
|
+ rc = 0;
|
|
+out:
|
|
+ if ( elf_check_broken(elf) )
|
|
+ DOMPRINTF("%s: ELF broken: %s", __FUNCTION__,
|
|
+ elf_check_broken(elf));
|
|
+
|
|
+ return rc;
|
|
}
|
|
|
|
static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
|
|
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
|
|
index ccfd8b5..8165287 100644
|
|
--- a/tools/libxc/xc_hvm_build_x86.c
|
|
+++ b/tools/libxc/xc_hvm_build_x86.c
|
|
@@ -403,11 +403,16 @@ static int setup_guest(xc_interface *xch,
|
|
munmap(page0, PAGE_SIZE);
|
|
}
|
|
|
|
+ if ( elf_check_broken(&elf) )
|
|
+ ERROR("HVM ELF broken: %s", elf_check_broken(&elf));
|
|
+
|
|
free(page_array);
|
|
return 0;
|
|
|
|
error_out:
|
|
free(page_array);
|
|
+ if ( elf_check_broken(&elf) )
|
|
+ ERROR("HVM ELF broken, failing: %s", elf_check_broken(&elf));
|
|
return -1;
|
|
}
|
|
|
|
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
|
|
index cfae994..d1f7a30 100644
|
|
--- a/tools/xcutils/readnotes.c
|
|
+++ b/tools/xcutils/readnotes.c
|
|
@@ -301,6 +301,9 @@ int main(int argc, char **argv)
|
|
printf("__xen_guest: %s\n",
|
|
elf_strfmt(&elf, elf_section_start(&elf, shdr)));
|
|
|
|
+ if (elf_check_broken(&elf))
|
|
+ printf("warning: broken ELF: %s\n", elf_check_broken(&elf));
|
|
+
|
|
return 0;
|
|
}
|
|
|
|
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
|
|
index 2d56130..dec0519 100644
|
|
--- a/xen/arch/arm/kernel.c
|
|
+++ b/xen/arch/arm/kernel.c
|
|
@@ -146,6 +146,8 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
|
|
{
|
|
int rc;
|
|
|
|
+ memset(&info->elf.elf, 0, sizeof(info->elf.elf));
|
|
+
|
|
info->kernel_order = get_order_from_bytes(KERNEL_FLASH_SIZE);
|
|
info->kernel_img = alloc_xenheap_pages(info->kernel_order, 0);
|
|
if ( info->kernel_img == NULL )
|
|
@@ -160,7 +162,7 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
|
|
#endif
|
|
elf_parse_binary(&info->elf.elf);
|
|
if ( (rc = elf_xen_parse(&info->elf.elf, &info->elf.parms)) != 0 )
|
|
- return rc;
|
|
+ goto err;
|
|
|
|
/*
|
|
* TODO: can the ELF header be used to find the physical address
|
|
@@ -169,7 +171,18 @@ static int kernel_try_elf_prepare(struct kernel_info *info)
|
|
info->entry = info->elf.parms.virt_entry;
|
|
info->load = kernel_elf_load;
|
|
|
|
+ if ( elf_check_broken(&info->elf.elf) )
|
|
+ printk("Xen: warning: ELF kernel broken: %s\n",
|
|
+ elf_check_broken(&info->elf.elf));
|
|
+
|
|
return 0;
|
|
+
|
|
+err:
|
|
+ if ( elf_check_broken(&info->elf.elf) )
|
|
+ printk("Xen: ELF kernel broken: %s\n",
|
|
+ elf_check_broken(&info->elf.elf));
|
|
+
|
|
+ return rc;
|
|
}
|
|
|
|
int kernel_prepare(struct kernel_info *info)
|
|
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
|
|
index a655b21..0dbec96 100644
|
|
--- a/xen/arch/x86/domain_build.c
|
|
+++ b/xen/arch/x86/domain_build.c
|
|
@@ -374,7 +374,7 @@ int __init construct_dom0(
|
|
#endif
|
|
elf_parse_binary(&elf);
|
|
if ( (rc = elf_xen_parse(&elf, &parms)) != 0 )
|
|
- return rc;
|
|
+ goto out;
|
|
|
|
/* compatibility check */
|
|
compatible = 0;
|
|
@@ -413,14 +413,16 @@ int __init construct_dom0(
|
|
if ( !compatible )
|
|
{
|
|
printk("Mismatch between Xen and DOM0 kernel\n");
|
|
- return -EINVAL;
|
|
+ rc = -EINVAL;
|
|
+ goto out;
|
|
}
|
|
|
|
if ( parms.elf_notes[XEN_ELFNOTE_SUPPORTED_FEATURES].type != XEN_ENT_NONE &&
|
|
!test_bit(XENFEAT_dom0, parms.f_supported) )
|
|
{
|
|
printk("Kernel does not support Dom0 operation\n");
|
|
- return -EINVAL;
|
|
+ rc = -EINVAL;
|
|
+ goto out;
|
|
}
|
|
|
|
#if defined(__x86_64__)
|
|
@@ -734,7 +736,8 @@ int __init construct_dom0(
|
|
(v_end > HYPERVISOR_COMPAT_VIRT_START(d)) )
|
|
{
|
|
printk("DOM0 image overlaps with Xen private area.\n");
|
|
- return -EINVAL;
|
|
+ rc = -EINVAL;
|
|
+ goto out;
|
|
}
|
|
|
|
if ( is_pv_32on64_domain(d) )
|
|
@@ -914,7 +917,7 @@ int __init construct_dom0(
|
|
if ( rc < 0 )
|
|
{
|
|
printk("Failed to load the kernel binary\n");
|
|
- return rc;
|
|
+ goto out;
|
|
}
|
|
bootstrap_map(NULL);
|
|
|
|
@@ -925,7 +928,8 @@ int __init construct_dom0(
|
|
{
|
|
write_ptbase(current);
|
|
printk("Invalid HYPERCALL_PAGE field in ELF notes.\n");
|
|
- return -1;
|
|
+ rc = -1;
|
|
+ goto out;
|
|
}
|
|
hypercall_page_initialise(
|
|
d, (void *)(unsigned long)parms.virt_hypercall);
|
|
@@ -1272,9 +1276,19 @@ int __init construct_dom0(
|
|
|
|
BUG_ON(rc != 0);
|
|
|
|
- iommu_dom0_init(dom0);
|
|
+ if ( elf_check_broken(&elf) )
|
|
+ printk(" Xen warning: dom0 kernel broken ELF: %s\n",
|
|
+ elf_check_broken(&elf));
|
|
|
|
+ iommu_dom0_init(dom0);
|
|
return 0;
|
|
+
|
|
+out:
|
|
+ if ( elf_check_broken(&elf) )
|
|
+ printk(" Xen dom0 kernel broken ELF: %s\n",
|
|
+ elf_check_broken(&elf));
|
|
+
|
|
+ return rc;
|
|
}
|
|
|
|
/*
|
|
--
|
|
1.7.2.5
|
|
|
|
|