This is version 5 of my to try to fix libelf and the domain loader.
Here is a summary of the state of series:
a 01/21 libelf: abolish libelf-relocate.c
* 02/21 libxc: introduce xc_dom_seg_to_ptr_pages
+ 03/21 libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
A 04/21 libelf: add `struct elf_binary*'' parameter to elf_load_image
a 05/21 libelf: abolish elf_sval and elf_access_signed
A 06/21 libelf: move include of <asm/guest_access.h> to top of file
a 07/21 libelf/xc_dom_load_elf_symtab: Do not use "syms"
uninitialised
08/21 libxl: introduce macros for memory access and pointer handling
a 09/21 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
A 10/21 libelf: check nul-terminated strings properly
* 11/21 libxl: check all pointer accesses
A 12/21 libxl: Check pointer references in elf_is_elfbinary
a- 13/21 libelf: Make all callers call elf_check_broken
a 14/21 libelf: use C99 bool for booleans
15/21 libelf: use only unsigned integers
* 16/21 libelf: check loops for running away
a 17/21 libelf: abolish obsolete macros
+ 18/21 libxc: Add range checking to xc_dom_binloader
+ 19/21 libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
+ 20/21 libxc: check return values from malloc
+ 21/21 libxc: range checks in xc_dom_p2m_host and _guest
Key to symbols:
* Updated in this version of the series.
+ New patch in this version.
/ Updated but only to remove changes into a separate patch.
- Updated with style changes only.
a Acked/reviwed by one reviewer.
A Acked/reviwed by more than one reviewer.
Also in every patch:
Updated commit msgs to correct email address for me.
libelf, and some of its callers, did not do nearly enough checking on
their input. Invalid inputs could cause signed integer arithmetic
overflows and wild pointer dereferences.
In this series we try to systematically eliminate this problem in a
way which has a reasonable chance of (i) still accepting all
previously-accepted ELF images (ii) not having remaining security
bugs, in a form which can be reviewied to verify (i) and (ii).
Additionally, we fix some related security bugs in the domain loading
code.
The approach is:
(i) Remove all uses of signed integers (of any kind). That
elmininates all integer overflows as sources of undefined
behaviour. Of course it still means that we can get incorrect
values throughout the code.
(ii) Replace all uses of pointers, both pointers into the supplied
ELF image, and pointers into the output (where we are loading)
by uintptr_t. That eliminates all pointer arithmetic overflows as
sources of undefined behaviour. Of course it still means that we
can get incorrect and unreasonable "pointer" values.
(iii) But these pointer values will be in uintptr_t, which cannot be
simply dereferenced by mistake. We will replace all dereferences
by macros which do range checking; out of range reads will read 0
and out of range writes will be ignored. Happily most (but not
all) of the reads in the code already go through macros which
abstract endianness[1] and/or 32/64bitness.
[1] Although not all the accesses use endian-aware techniques so
in fact the code can''t cope with foreign-endian ELFs. This is a
problem for another day.
(iv) Look for all loops and check that they are guaranteed to
terminate.
To enable verification of correctness of these changes I provide them
as a series roughly as follows:
1-6:
Pre-patches which make a few semantically neutral or semantically
correct changes. For human review.
7: Introduces a set of macros to abstract away pointer arithmetic and
input and output image memory accesses in libelf. Use these macros
everwhere they are applicable. However, define the macros in a way
that corresponds to the existing code. That this patch has no
functional change can be verified by comparing the before-and-after
assembler output from the compiler.
9. Introduce some macros for dealing with nul-terminated strings,
defined so as not to have any functional change at this stage.
10. Change the macro definitions, and introduce the new pseudopointer
types, pointer range checking, etc. For close human review. Each
macro change is justified in the commit message. This patch
eliminates most of the potential wild pointer accesses.
8,11-12,14-15:
Smaller patches for human review, fixing some leftover bugs,
including ensuring that all loops terminate.
13: Eliminate signed integers. Replace every "int",
"int*_t",
"long" and most "char"s by corresponding unsigned types.
This
eliminates all integer arithmetic overflows.
After this patch, libelf should be safe against hostile input:
* All arithmetic operations on values from the input file use
unsigned arithmetic which is guaranteed to be defined (although
it may of course result in wrong answers);
* All pointer accesses based on pointers to locations which depend on
the input file go via our range-checking accessors; accesses which
are not to the input or output regions are ignored (reads returning
0).
* The loops have been checked to ensure that they terminate and are at
worst O(image_size).
* Whenever an array variable was declared, the code has been manually
reviewed looking for possible out-of-bounds accesses.
This is XSA-55.
This file is not actually used. It''s not built in Xen''s
instance of
libelf; in libxc''s it''s built but nothing in it is called. Do
not
compile it in libxc, and delete it.
This reduces the amount of work we need to do in forthcoming patches
to libelf (particularly since as libelf-relocate.c is not used it is
probably full of bugs).
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>
---
tools/libxc/Makefile | 2 +-
xen/common/libelf/libelf-relocate.c | 372 -----------------------------------
2 files changed, 1 insertions(+), 373 deletions(-)
delete mode 100644 xen/common/libelf/libelf-relocate.c
diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index b200123..4a31282 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -52,7 +52,7 @@ vpath %.c ../../xen/common/libelf
CFLAGS += -I../../xen/common/libelf
GUEST_SRCS-y += libelf-tools.c libelf-loader.c
-GUEST_SRCS-y += libelf-dominfo.c libelf-relocate.c
+GUEST_SRCS-y += libelf-dominfo.c
# new domain builder
GUEST_SRCS-y += xc_dom_core.c xc_dom_boot.c
diff --git a/xen/common/libelf/libelf-relocate.c
b/xen/common/libelf/libelf-relocate.c
deleted file mode 100644
index 2aafc44..0000000
--- a/xen/common/libelf/libelf-relocate.c
+++ /dev/null
@@ -1,372 +0,0 @@
-/*
- * ELF relocation code (not used by xen kernel right now).
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation;
- * version 2.1 of the License.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
- */
-
-#include "libelf-private.h"
-
-/* ------------------------------------------------------------------------ */
-
-static const char *rel_names_i386[] = {
- "R_386_NONE",
- "R_386_32",
- "R_386_PC32",
- "R_386_GOT32",
- "R_386_PLT32",
- "R_386_COPY",
- "R_386_GLOB_DAT",
- "R_386_JMP_SLOT",
- "R_386_RELATIVE",
- "R_386_GOTOFF",
- "R_386_GOTPC",
- "R_386_32PLT",
- "R_386_TLS_TPOFF",
- "R_386_TLS_IE",
- "R_386_TLS_GOTIE",
- "R_386_TLS_LE",
- "R_386_TLS_GD",
- "R_386_TLS_LDM",
- "R_386_16",
- "R_386_PC16",
- "R_386_8",
- "R_386_PC8",
- "R_386_TLS_GD_32",
- "R_386_TLS_GD_PUSH",
- "R_386_TLS_GD_CALL",
- "R_386_TLS_GD_POP",
- "R_386_TLS_LDM_32",
- "R_386_TLS_LDM_PUSH",
- "R_386_TLS_LDM_CALL",
- "R_386_TLS_LDM_POP",
- "R_386_TLS_LDO_32",
- "R_386_TLS_IE_32",
- "R_386_TLS_LE_32",
- "R_386_TLS_DTPMOD32",
- "R_386_TLS_DTPOFF32",
- "R_386_TLS_TPOFF32",
-};
-
-static int elf_reloc_i386(struct elf_binary *elf, int type,
- uint64_t addr, uint64_t value)
-{
- void *ptr = elf_get_ptr(elf, addr);
- uint32_t *u32;
-
- switch ( type )
- {
- case 1 /* R_386_32 */ :
- u32 = ptr;
- *u32 += elf->reloc_offset;
- break;
- case 2 /* R_386_PC32 */ :
- /* nothing */
- break;
- default:
- return -1;
- }
- return 0;
-}
-
-/* ------------------------------------------------------------------------ */
-
-static const char *rel_names_x86_64[] = {
- "R_X86_64_NONE",
- "R_X86_64_64",
- "R_X86_64_PC32",
- "R_X86_64_GOT32",
- "R_X86_64_PLT32",
- "R_X86_64_COPY",
- "R_X86_64_GLOB_DAT",
- "R_X86_64_JUMP_SLOT",
- "R_X86_64_RELATIVE",
- "R_X86_64_GOTPCREL",
- "R_X86_64_32",
- "R_X86_64_32S",
- "R_X86_64_16",
- "R_X86_64_PC16",
- "R_X86_64_8",
- "R_X86_64_PC8",
- "R_X86_64_DTPMOD64",
- "R_X86_64_DTPOFF64",
- "R_X86_64_TPOFF64",
- "R_X86_64_TLSGD",
- "R_X86_64_TLSLD",
- "R_X86_64_DTPOFF32",
- "R_X86_64_GOTTPOFF",
- "R_X86_64_TPOFF32",
-};
-
-static int elf_reloc_x86_64(struct elf_binary *elf, int type,
- uint64_t addr, uint64_t value)
-{
- void *ptr = elf_get_ptr(elf, addr);
- uint64_t *u64;
- uint32_t *u32;
- int32_t *s32;
-
- switch ( type )
- {
- case 1 /* R_X86_64_64 */ :
- u64 = ptr;
- value += elf->reloc_offset;
- *u64 = value;
- break;
- case 2 /* R_X86_64_PC32 */ :
- u32 = ptr;
- *u32 = value - addr;
- if ( *u32 != (uint32_t)(value - addr) )
- {
- elf_err(elf, "R_X86_64_PC32 overflow: 0x%" PRIx32
- " != 0x%" PRIx32 "\n",
- *u32, (uint32_t) (value - addr));
- return -1;
- }
- break;
- case 10 /* R_X86_64_32 */ :
- u32 = ptr;
- value += elf->reloc_offset;
- *u32 = value;
- if ( *u32 != value )
- {
- elf_err(elf, "R_X86_64_32 overflow: 0x%" PRIx32
- " != 0x%" PRIx64 "\n",
- *u32, value);
- return -1;
- }
- break;
- case 11 /* R_X86_64_32S */ :
- s32 = ptr;
- value += elf->reloc_offset;
- *s32 = value;
- if ( *s32 != (int64_t) value )
- {
- elf_err(elf, "R_X86_64_32S overflow: 0x%" PRIx32
- " != 0x%" PRIx64 "\n",
- *s32, (int64_t) value);
- return -1;
- }
- break;
- default:
- return -1;
- }
- return 0;
-}
-
-/* ------------------------------------------------------------------------ */
-
-static struct relocs {
- const char **names;
- int count;
- int (*func) (struct elf_binary * elf, int type, uint64_t addr,
- uint64_t value);
-} relocs[] -/* *INDENT-OFF* */
-{
- [EM_386] = {
- .names = rel_names_i386,
- .count = sizeof(rel_names_i386) / sizeof(rel_names_i386[0]),
- .func = elf_reloc_i386,
- },
- [EM_X86_64] = {
- .names = rel_names_x86_64,
- .count = sizeof(rel_names_x86_64) / sizeof(rel_names_x86_64[0]),
- .func = elf_reloc_x86_64,
- }
-};
-/* *INDENT-ON* */
-
-/* ------------------------------------------------------------------------ */
-
-static const char *rela_name(int machine, int type)
-{
- if ( machine > sizeof(relocs) / sizeof(relocs[0]) )
- return "unknown mach";
- if ( !relocs[machine].names )
- return "unknown mach";
- if ( type > relocs[machine].count )
- return "unknown rela";
- return relocs[machine].names[type];
-}
-
-static int elf_reloc_section(struct elf_binary *elf,
- const elf_shdr * rels,
- const elf_shdr * sect, const elf_shdr * syms)
-{
- const void *ptr, *end;
- const elf_shdr *shdr;
- const elf_rela *rela;
- const elf_rel *rel;
- const elf_sym *sym;
- uint64_t s_type;
- uint64_t r_offset;
- uint64_t r_info;
- uint64_t r_addend;
- int r_type, r_sym;
- size_t rsize;
- uint64_t shndx, sbase, addr, value;
- const char *sname;
- int machine;
-
- machine = elf_uval(elf, elf->ehdr, e_machine);
- if ( (machine >= (sizeof(relocs) / sizeof(relocs[0]))) ||
- (relocs[machine].func == NULL) )
- {
- elf_err(elf, "%s: can''t handle machine %d\n",
- __FUNCTION__, machine);
- return -1;
- }
- if ( elf_swap(elf) )
- {
- elf_err(elf, "%s: non-native byte order, relocation not
supported\n",
- __FUNCTION__);
- return -1;
- }
-
- s_type = elf_uval(elf, rels, sh_type);
- rsize = (SHT_REL == s_type) ? elf_size(elf, rel) : elf_size(elf, rela);
- ptr = elf_section_start(elf, rels);
- end = elf_section_end(elf, rels);
-
- for ( ; ptr < end; ptr += rsize )
- {
- switch ( s_type )
- {
- case SHT_REL:
- rel = ptr;
- r_offset = elf_uval(elf, rel, r_offset);
- r_info = elf_uval(elf, rel, r_info);
- r_addend = 0;
- break;
- case SHT_RELA:
- rela = ptr;
- r_offset = elf_uval(elf, rela, r_offset);
- r_info = elf_uval(elf, rela, r_info);
- r_addend = elf_uval(elf, rela, r_addend);
- break;
- default:
- /* can''t happen */
- return -1;
- }
- if ( elf_64bit(elf) )
- {
- r_type = ELF64_R_TYPE(r_info);
- r_sym = ELF64_R_SYM(r_info);
- }
- else
- {
- r_type = ELF32_R_TYPE(r_info);
- r_sym = ELF32_R_SYM(r_info);
- }
-
- sym = elf_sym_by_index(elf, r_sym);
- shndx = elf_uval(elf, sym, st_shndx);
- switch ( shndx )
- {
- case SHN_UNDEF:
- sname = "*UNDEF*";
- sbase = 0;
- break;
- case SHN_COMMON:
- elf_err(elf, "%s: invalid section: %" PRId64
"\n",
- __FUNCTION__, shndx);
- return -1;
- case SHN_ABS:
- sname = "*ABS*";
- sbase = 0;
- break;
- default:
- shdr = elf_shdr_by_index(elf, shndx);
- if ( shdr == NULL )
- {
- elf_err(elf, "%s: invalid section: %" PRId64
"\n",
- __FUNCTION__, shndx);
- return -1;
- }
- sname = elf_section_name(elf, shdr);
- sbase = elf_uval(elf, shdr, sh_addr);
- }
-
- addr = r_offset;
- value = elf_uval(elf, sym, st_value);
- value += r_addend;
-
- if ( elf->log_callback && (elf->verbose > 1) )
- {
- uint64_t st_name = elf_uval(elf, sym, st_name);
- const char *name = st_name ? elf->sym_strtab + st_name :
"*NONE*";
-
- elf_msg(elf,
- "%s: type %s [%d], off 0x%" PRIx64 ", add
0x%" PRIx64 ","
- " sym %s [0x%" PRIx64 "], sec %s [0x%"
PRIx64 "]"
- " -> addr 0x%" PRIx64 " value 0x%"
PRIx64 "\n",
- __FUNCTION__, rela_name(machine, r_type), r_type, r_offset,
- r_addend, name, elf_uval(elf, sym, st_value), sname, sbase,
- addr, value);
- }
-
- if ( relocs[machine].func(elf, r_type, addr, value) == -1 )
- {
- elf_err(elf, "%s: unknown/unsupported reloc type %s
[%d]\n",
- __FUNCTION__, rela_name(machine, r_type), r_type);
- return -1;
- }
- }
- return 0;
-}
-
-int elf_reloc(struct elf_binary *elf)
-{
- const elf_shdr *rels, *sect, *syms;
- uint64_t i, count, type;
-
- count = elf_shdr_count(elf);
- for ( i = 0; i < count; i++ )
- {
- rels = elf_shdr_by_index(elf, i);
- type = elf_uval(elf, rels, sh_type);
- if ( (type != SHT_REL) && (type != SHT_RELA) )
- continue;
-
- sect = elf_shdr_by_index(elf, elf_uval(elf, rels, sh_info));
- syms = elf_shdr_by_index(elf, elf_uval(elf, rels, sh_link));
- if ( NULL == sect || NULL == syms )
- continue;
-
- if ( !(elf_uval(elf, sect, sh_flags) & SHF_ALLOC) )
- {
- elf_msg(elf, "%s: relocations for %s, skipping\n",
- __FUNCTION__, elf_section_name(elf, sect));
- continue;
- }
-
- elf_msg(elf, "%s: relocations for %s @ 0x%" PRIx64
"\n",
- __FUNCTION__, elf_section_name(elf, sect),
- elf_uval(elf, sect, sh_addr));
- if ( elf_reloc_section(elf, rels, sect, syms) != 0 )
- return -1;
- }
- return 0;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
--
1.7.2.5
Provide a version of xc_dom_seg_to_ptr which returns the number of
guest pages it has actually mapped. This is useful for callers who
want to do range checking; we will use this later in this series.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: xc_dom_seg_to_ptr_pages sets *pages_out=0 if it returns NULL.
v4 was:
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/xc_dom.h | 18 +++++++++++++++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index ac36600..da0598c 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -294,14 +294,26 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom,
xen_pfn_t first,
void xc_dom_unmap_one(struct xc_dom_image *dom, xen_pfn_t pfn);
void xc_dom_unmap_all(struct xc_dom_image *dom);
-static inline void *xc_dom_seg_to_ptr(struct xc_dom_image *dom,
- struct xc_dom_seg *seg)
+static inline void *xc_dom_seg_to_ptr_pages(struct xc_dom_image *dom,
+ struct xc_dom_seg *seg,
+ xen_pfn_t *pages_out)
{
xen_vaddr_t segsize = seg->vend - seg->vstart;
unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
xen_pfn_t pages = (segsize + page_size - 1) / page_size;
+ void *retval;
+
+ retval = xc_dom_pfn_to_ptr(dom, seg->pfn, pages);
+
+ if ( pages_out != NULL )
+ *pages_out = retval ? pages : 0;
+ return retval;
+}
- return xc_dom_pfn_to_ptr(dom, seg->pfn, pages);
+static inline void *xc_dom_seg_to_ptr(struct xc_dom_image *dom,
+ struct xc_dom_seg *seg)
+{
+ return xc_dom_seg_to_ptr_pages(dom, seg, NULL);
}
static inline void *xc_dom_vaddr_to_ptr(struct xc_dom_image *dom,
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 03/21] libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
* Ensure that xc_dom_pfn_to_ptr (when called with count==0) does not
return a previously-allocated block which is entirely before the
requested pfn (!)
* Provide a version of xc_dom_pfn_to_ptr, xc_dom_pfn_to_ptr_retcount,
which provides the length of the mapped region via an out parameter.
* Change xc_dom_vaddr_to_ptr to always provide the length of the
mapped region and change the call site in xc_dom_binloader.c to
check it. The call site in xc_dom_load_elf_symtab will be corrected
in a forthcoming patch, and for now ignores the returned length.
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_dom.h | 16 +++++++++++++---
tools/libxc/xc_dom_binloader.c | 11 ++++++++++-
tools/libxc/xc_dom_core.c | 13 +++++++++++++
tools/libxc/xc_dom_elfloader.c | 3 ++-
4 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index da0598c..c7eaf85 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -291,6 +291,8 @@ int xc_dom_alloc_segment(struct xc_dom_image *dom,
void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t first,
xen_pfn_t count);
+void *xc_dom_pfn_to_ptr_retcount(struct xc_dom_image *dom, xen_pfn_t first,
+ xen_pfn_t count, xen_pfn_t *count_out);
void xc_dom_unmap_one(struct xc_dom_image *dom, xen_pfn_t pfn);
void xc_dom_unmap_all(struct xc_dom_image *dom);
@@ -317,13 +319,21 @@ static inline void *xc_dom_seg_to_ptr(struct xc_dom_image
*dom,
}
static inline void *xc_dom_vaddr_to_ptr(struct xc_dom_image *dom,
- xen_vaddr_t vaddr)
+ xen_vaddr_t vaddr,
+ size_t *safe_region_out)
{
unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
xen_pfn_t page = (vaddr - dom->parms.virt_base) / page_size;
unsigned int offset = (vaddr - dom->parms.virt_base) % page_size;
- void *ptr = xc_dom_pfn_to_ptr(dom, page, 0);
- return (ptr ? (ptr + offset) : NULL);
+ xen_pfn_t safe_region_count;
+ void *ptr;
+
+ *safe_region_out = 0;
+ ptr = xc_dom_pfn_to_ptr_retcount(dom, page, 0, &safe_region_count);
+ if ( ptr == NULL )
+ return ptr;
+ *safe_region_out = (safe_region_count << XC_DOM_PAGE_SHIFT(dom)) -
offset;
+ return ptr;
}
static inline xen_pfn_t xc_dom_p2m_host(struct xc_dom_image *dom, xen_pfn_t
pfn)
diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index c14727c..d2de04c 100644
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -249,6 +249,7 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
char *image = dom->kernel_blob;
char *dest;
size_t image_size = dom->kernel_size;
+ size_t dest_size;
uint32_t start_addr;
uint32_t load_end_addr;
uint32_t bss_end_addr;
@@ -272,7 +273,15 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
DOMPRINTF(" text_size: 0x%" PRIx32 "", text_size);
DOMPRINTF(" bss_size: 0x%" PRIx32 "", bss_size);
- dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart);
+ dest = xc_dom_vaddr_to_ptr(dom, dom->kernel_seg.vstart, &dest_size);
+
+ if ( dest_size < text_size ||
+ dest_size - text_size < bss_size )
+ {
+ DOMPRINTF("%s: mapped region is too small for image",
__FUNCTION__);
+ return -EINVAL;
+ }
+
memcpy(dest, image + skip, text_size);
memset(dest + text_size, 0, bss_size);
diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
index b92e4a9..cf96bfa 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -351,11 +351,20 @@ int xc_dom_try_gunzip(struct xc_dom_image *dom, void
**blob, size_t * size)
void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t pfn,
xen_pfn_t count)
{
+ xen_pfn_t count_out_dummy;
+ return xc_dom_pfn_to_ptr_retcount(dom, pfn, count, &count_out_dummy);
+}
+
+void *xc_dom_pfn_to_ptr_retcount(struct xc_dom_image *dom, xen_pfn_t pfn,
+ xen_pfn_t count, xen_pfn_t *count_out)
+{
struct xc_dom_phys *phys;
xen_pfn_t offset;
unsigned int page_shift = XC_DOM_PAGE_SHIFT(dom);
char *mode = "unset";
+ *count_out = 0;
+
offset = pfn - dom->rambase_pfn;
if ( offset > dom->total_pages || /* multiple checks to avoid
overflows */
count > dom->total_pages ||
@@ -386,6 +395,7 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t
pfn,
phys->count);
return NULL;
}
+ *count_out = count;
}
else
{
@@ -393,6 +403,9 @@ void *xc_dom_pfn_to_ptr(struct xc_dom_image *dom, xen_pfn_t
pfn,
just hand out a pointer to it */
if ( pfn < phys->first )
continue;
+ if ( pfn >= phys->first + phys->count )
+ continue;
+ *count_out = phys->count - (pfn - phys->first);
}
return phys->ptr + ((pfn - phys->first) << page_shift);
}
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 6583859..bc92302 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -128,10 +128,11 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
if ( load )
{
+ size_t allow_size; /* will be used in a forthcoming XSA-55 patch */
if ( !dom->bsd_symtab_start )
return 0;
size = dom->kernel_seg.vend - dom->bsd_symtab_start;
- hdr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start);
+ hdr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start,
&allow_size);
*(int *)hdr = size - sizeof(int);
}
else
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 04/21] libelf: add `struct elf_binary*'' parameter to elf_load_image
The meat of this function is going to need a copy of the elf pointer,
in forthcoming patches.
No functional change in this patch.
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>
---
xen/common/libelf/libelf-loader.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 3cf9c59..bce667f 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -108,7 +108,8 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback
*log_callback,
elf->verbose = verbose;
}
-static int elf_load_image(void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
+static int elf_load_image(struct elf_binary *elf,
+ void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
{
memcpy(dst, src, filesz);
memset(dst + filesz, 0, memsz - filesz);
@@ -122,7 +123,8 @@ void elf_set_verbose(struct elf_binary *elf)
elf->verbose = 1;
}
-static int elf_load_image(void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
+static int elf_load_image(struct elf_binary *elf,
+ void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
{
int rc;
if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
@@ -279,7 +281,7 @@ int elf_load_binary(struct elf_binary *elf)
dest = elf_get_ptr(elf, paddr);
elf_msg(elf, "%s: phdr %" PRIu64 " at 0x%p ->
0x%p\n",
__func__, i, dest, dest + filesz);
- if ( elf_load_image(dest, elf->image + offset, filesz, memsz) != 0 )
+ if ( elf_load_image(elf, dest, elf->image + offset, filesz, memsz)
!= 0 )
return -1;
}
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 05/21] libelf: abolish elf_sval and elf_access_signed
These are not used anywhere.
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>
---
xen/common/libelf/libelf-tools.c | 28 ----------------------------
xen/include/xen/libelf.h | 11 -----------
2 files changed, 0 insertions(+), 39 deletions(-)
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 8312065..1f08407 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -48,34 +48,6 @@ uint64_t elf_access_unsigned(struct elf_binary * elf, const
void *ptr,
}
}
-int64_t elf_access_signed(struct elf_binary *elf, const void *ptr,
- uint64_t offset, size_t size)
-{
- int need_swap = elf_swap(elf);
- const int8_t *s8;
- const int16_t *s16;
- const int32_t *s32;
- const int64_t *s64;
-
- switch ( size )
- {
- case 1:
- s8 = ptr + offset;
- return *s8;
- case 2:
- s16 = ptr + offset;
- return need_swap ? bswap_16(*s16) : *s16;
- case 4:
- s32 = ptr + offset;
- return need_swap ? bswap_32(*s32) : *s32;
- case 8:
- s64 = ptr + offset;
- return need_swap ? bswap_64(*s64) : *s64;
- default:
- return 0;
- }
-}
-
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr)
{
int elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 218bb18..ae03982 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -136,23 +136,12 @@ struct elf_binary {
offsetof(typeof(*(str)),e32.elem), \
sizeof((str)->e32.elem)))
-#define elf_sval(elf, str, elem) \
- ((ELFCLASS64 == (elf)->class) \
- ? elf_access_signed((elf), (str), \
- offsetof(typeof(*(str)),e64.elem), \
- sizeof((str)->e64.elem)) \
- : elf_access_signed((elf), (str), \
- offsetof(typeof(*(str)),e32.elem), \
- sizeof((str)->e32.elem)))
-
#define elf_size(elf, str) \
((ELFCLASS64 == (elf)->class) \
? sizeof((str)->e64) : sizeof((str)->e32))
uint64_t elf_access_unsigned(struct elf_binary *elf, const void *ptr,
uint64_t offset, size_t size);
-int64_t elf_access_signed(struct elf_binary *elf, const void *ptr,
- uint64_t offset, size_t size);
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 06/21] libelf: move include of <asm/guest_access.h> to top of file
libelf-loader.c #includes <asm/guest_access.h>, when being compiled
for Xen. Currently it does this in the middle of the file.
Move this #include to the top of the file, before libelf-private.h.
This is necessary because in forthcoming patches we will introduce
private #defines of memcpy etc. which would interfere with definitions
in headers #included from guest_access.h.
No semantic or functional change in this patch.
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>
---
xen/common/libelf/libelf-loader.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index bce667f..94257f6 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -16,6 +16,10 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA
*/
+#ifdef __XEN__
+#include <asm/guest_access.h>
+#endif
+
#include "libelf-private.h"
/* ------------------------------------------------------------------------ */
@@ -116,7 +120,6 @@ static int elf_load_image(struct elf_binary *elf,
return 0;
}
#else
-#include <asm/guest_access.h>
void elf_set_verbose(struct elf_binary *elf)
{
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 07/21] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
xc_dom_load_elf_symtab (with load==0) calls elf_round_up, but it
mistakenly used the uninitialised variable "syms" when calculating
dom->bsd_symtab_start. This should be a reference to "elf".
This change might have the effect of rounding the value differently.
Previously if the uninitialised value (a single byte on the stack) was
ELFCLASS64 (ie, 2), the alignment would be to 8 bytes, otherwise to 4.
However, the value is calculated from dom->kernel_seg.vend so this
could only make a difference if that value wasn''t already aligned to 8
bytes.
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>
v2: Split this change into its own patch for proper review.
---
tools/libxc/xc_dom_elfloader.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index bc92302..7ff51d1 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -142,7 +142,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
hdr = xc_dom_malloc(dom, size);
if ( hdr == NULL )
return 0;
- dom->bsd_symtab_start = elf_round_up(&syms,
dom->kernel_seg.vend);
+ dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
}
memcpy(hdr + sizeof(int),
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 08/21] libelf: introduce macros for memory access and pointer handling
We introduce a collection of macros which abstract away all the
pointer arithmetic and dereferences used for accessing the input ELF
and the output area(s). We use the new macros everywhere.
For now, these macros are semantically identical to the code they
replace, so this patch has no functional change.
elf_is_elfbinary is an exception: since it doesn''t take an elf*, we
need to handle it differently. In a future patch we will change it to
take, and check, a length parameter. For now we just mark it with a
fixme.
That this patch has no functional change can be verified as follows:
0. Copy the scripts "comparison-generate" and
"function-filter"
out of this commit message.
1. Check out the tree before this patch.
2. Run the script ../comparison-generate .... ../before
3. Check out the tree after this patch.
4. Run the script ../comparison-generate .... ../after
5. diff --exclude=\*.[soi] -ruN before/ after/ |less
Expect these differences:
* stubdom/zlib-x86_64/ztest*.s2
The filename of this test file apparently contains the pid.
* xen/common/version.s2
The xen build timestamp appears in two diff hunks.
Verification that this is all that''s needed:
In a completely built xen.git,
find * -name .*.d -type f | xargs grep -l libelf\.h
Expect results in:
xen/arch/x86: Checked above.
tools/libxc: Checked above.
tools/xcutils/readnotes: Checked above.
tools/xenstore: Checked above.
xen/common/libelf:
This is the build for the hypervisor; checked in B above.
stubdom:
We have one stubdom which reads ELFs using our libelf,
pvgrub, which is checked above.
I have not done this verification for ARM.
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>
v4: Fix elf_load_binary''s phdr message to be correct on 32-bit.
Fix ELF_OBSOLETE_VOIDP_CAST to work on 32-bit.
Indent scripts in commit message.
v3.1:
Change elf_store_field to verify correctly on 32-bit.
comparison-generate copes with Xen 4.1''s lack of ./configure.
v2: Use Xen style for multi-line comments.
Postpone changes to readnotes.c:print_l1_mfn_valid_note.
Much improved verification instructions with new script.
Fixed commit message subject.
-8<- comparison-generate -8<-
#!/bin/bash
# usage:
# cd xen.git
# .../comparison-generate OUR-CONFIG BUILD-RUNE-PREFIX ../before|../after
# eg:
# .../comparison-generate ~/work/.config ''schroot -pc64 --''
../before
set -ex
test $# = 3 || need-exactly-three-arguments
our_config=$1
build_rune_prefix=$2
result_dir=$3
git-clean -x -d -f
cp "$our_config" .
cat <<END >>.config
debug_symbols=n
CFLAGS += -save-temps
END
perl -i~ -pe ''s/ -g / -g0 / if m/^CFLAGS/'' xen/Rules.mk
if [ -f ./configure ]; then
$build_rune_prefix ./configure
fi
$build_rune_prefix make -C xen
$build_rune_prefix make -C tools/include
$build_rune_prefix make -C stubdom grub
$build_rune_prefix make -C tools/libxc
$build_rune_prefix make -C tools/xenstore
$build_rune_prefix make -C tools/xcutils
rm -rf "$result_dir"
mkdir "$result_dir"
set +x
for f in `find xen tools stubdom -name \*.[soi]`; do
mkdir -p "$result_dir"/`dirname $f`
cp $f "$result_dir"/${f}
case $f in
*.s)
../function-filter <$f >"$result_dir"/${f}2
;;
esac
done
echo ok.
-8<-
-8<- function-filter -8<-
#!/usr/bin/perl -w
# function-filter
# script for massaging gcc-generated labels to be consistent
use strict;
our @lines;
my $sedderybody = "sub seddery () {\n";
while (<>) {
push @lines, $_;
if (m/^(__FUNCTION__|__func__)\.(\d+)\:/) {
$sedderybody .= "
s/\\b$1\\.$2\\b/__XSA55MANGLED__$1.$./g;\n";
}
}
$sedderybody .= "}\n1;\n";
eval $sedderybody or die $@;
foreach (@lines) {
seddery();
print or die $!;
}
-8<-
---
tools/libxc/xc_dom_elfloader.c | 30 +++---
tools/libxc/xc_hvm_build_x86.c | 2 +-
tools/xcutils/readnotes.c | 26 +++---
xen/common/libelf/libelf-dominfo.c | 51 +++++-----
xen/common/libelf/libelf-loader.c | 84 +++++++++--------
xen/common/libelf/libelf-tools.c | 94 +++++++++---------
xen/include/xen/libelf.h | 188 +++++++++++++++++++++++++++++++-----
7 files changed, 312 insertions(+), 163 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 7ff51d1..b8089bc 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -113,9 +113,9 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
struct elf_binary *elf, int load)
{
struct elf_binary syms;
- const elf_shdr *shdr, *shdr2;
+ ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr; ELF_HANDLE_DECL(elf_shdr) shdr2;
xen_vaddr_t symtab, maxaddr;
- char *hdr;
+ ELF_PTRVAL_CHAR hdr;
size_t size;
int h, count, type, i, tables = 0;
@@ -145,11 +145,11 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
}
- memcpy(hdr + sizeof(int),
- elf->image,
+ elf_memcpy_safe(elf, hdr + sizeof(int),
+ ELF_IMAGE_BASE(elf),
elf_size(elf, elf->ehdr));
- memcpy(hdr + sizeof(int) + elf_size(elf, elf->ehdr),
- elf->image + elf_uval(elf, elf->ehdr, e_shoff),
+ elf_memcpy_safe(elf, hdr + sizeof(int) + elf_size(elf, elf->ehdr),
+ ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
elf_shdr_count(elf) * elf_size(elf, shdr));
if ( elf_64bit(elf) )
{
@@ -187,7 +187,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
count = elf_shdr_count(&syms);
for ( h = 0; h < count; h++ )
{
- shdr = elf_shdr_by_index(&syms, h);
+ shdr = ELF_OBSOLETE_VOIDP_CAST elf_shdr_by_index(&syms, h);
type = elf_uval(&syms, shdr, sh_type);
if ( type == SHT_STRTAB )
{
@@ -203,9 +203,9 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
if ( i == count )
{
if ( elf_64bit(&syms) )
- *(Elf64_Off*)(&shdr->e64.sh_offset) = 0;
+ elf_store_field(elf, shdr, e64.sh_offset, 0);
else
- *(Elf32_Off*)(&shdr->e32.sh_offset) = 0;
+ elf_store_field(elf, shdr, e32.sh_offset, 0);
continue;
}
}
@@ -214,9 +214,9 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
{
/* Mangled to be based on ELF header location. */
if ( elf_64bit(&syms) )
- *(Elf64_Off*)(&shdr->e64.sh_offset) = maxaddr - symtab;
+ elf_store_field(elf, shdr, e64.sh_offset, maxaddr - symtab);
else
- *(Elf32_Off*)(&shdr->e32.sh_offset) = maxaddr - symtab;
+ elf_store_field(elf, shdr, e32.sh_offset, maxaddr - symtab);
size = elf_uval(&syms, shdr, sh_size);
maxaddr = elf_round_up(&syms, maxaddr + size);
tables++;
@@ -228,7 +228,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
if ( load )
{
shdr2 = elf_shdr_by_index(elf, h);
- memcpy((void*)elf_section_start(&syms, shdr),
+ elf_memcpy_safe(elf, ELF_OBSOLETE_VOIDP_CAST
elf_section_start(&syms, shdr),
elf_section_start(elf, shdr2),
size);
}
@@ -236,9 +236,9 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
/* Name is NULL. */
if ( elf_64bit(&syms) )
- *(Elf64_Word*)(&shdr->e64.sh_name) = 0;
+ elf_store_field(elf, shdr, e64.sh_name, 0);
else
- *(Elf32_Word*)(&shdr->e32.sh_name) = 0;
+ elf_store_field(elf, shdr, e32.sh_name, 0);
}
if ( tables == 0 )
@@ -273,7 +273,7 @@ static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
}
/* Find the section-header strings table. */
- if ( elf->sec_strtab == NULL )
+ if ( ELF_PTRVAL_INVALID(elf->sec_strtab) )
{
xc_dom_panic(dom->xch, XC_INVALID_KERNEL, "%s: ELF image"
" has no shstrtab", __FUNCTION__);
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index ab33a7f..39f93a3 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -143,7 +143,7 @@ static int loadelfimage(xc_interface *xch, struct elf_binary
*elf,
if ( elf->dest == NULL )
goto err;
- elf->dest += elf->pstart & (PAGE_SIZE - 1);
+ ELF_ADVANCE_DEST(elf, elf->pstart & (PAGE_SIZE - 1));
/* Load the initial elf image. */
rc = elf_load_binary(elf);
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index c926186..2af047d 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -61,13 +61,13 @@ struct setup_header {
} __attribute__((packed));
static void print_string_note(const char *prefix, struct elf_binary *elf,
- const elf_note *note)
+ ELF_HANDLE_DECL(elf_note) note)
{
printf("%s: %s\n", prefix, (char*)elf_note_desc(elf, note));
}
static void print_numeric_note(const char *prefix, struct elf_binary *elf,
- const elf_note *note)
+ ELF_HANDLE_DECL(elf_note) note)
{
uint64_t value = elf_note_numeric(elf, note);
int descsz = elf_uval(elf, note, descsz);
@@ -98,12 +98,12 @@ static void print_l1_mfn_valid_note(const char *prefix,
struct elf_binary *elf,
}
-static int print_notes(struct elf_binary *elf, const elf_note *start, const
elf_note *end)
+static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start,
ELF_HANDLE_DECL(elf_note) end)
{
- const elf_note *note;
+ ELF_HANDLE_DECL(elf_note) note;
int notes_found = 0;
- for ( note = start; note < end; note = elf_note_next(elf, note) )
+ for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end); note
= elf_note_next(elf, note) )
{
if (0 != strcmp(elf_note_name(elf, note), "Xen"))
continue;
@@ -170,7 +170,7 @@ int main(int argc, char **argv)
void *image,*tmp;
struct stat st;
struct elf_binary elf;
- const elf_shdr *shdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
int notes_found = 0;
struct setup_header *hdr;
@@ -257,7 +257,7 @@ int main(int argc, char **argv)
count = elf_phdr_count(&elf);
for ( h=0; h < count; h++)
{
- const elf_phdr *phdr;
+ ELF_HANDLE_DECL(elf_phdr) phdr;
phdr = elf_phdr_by_index(&elf, h);
if (elf_uval(&elf, phdr, p_type) != PT_NOTE)
continue;
@@ -269,8 +269,8 @@ int main(int argc, char **argv)
continue;
notes_found = print_notes(&elf,
- elf_segment_start(&elf, phdr),
- elf_segment_end(&elf, phdr));
+ ELF_MAKE_HANDLE(elf_note, elf_segment_start(&elf, phdr)),
+ ELF_MAKE_HANDLE(elf_note, elf_segment_end(&elf, phdr)));
}
if ( notes_found == 0 )
@@ -278,13 +278,13 @@ int main(int argc, char **argv)
count = elf_shdr_count(&elf);
for ( h=0; h < count; h++)
{
- const elf_shdr *shdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
shdr = elf_shdr_by_index(&elf, h);
if (elf_uval(&elf, shdr, sh_type) != SHT_NOTE)
continue;
notes_found = print_notes(&elf,
- elf_section_start(&elf, shdr),
- elf_section_end(&elf, shdr));
+ ELF_MAKE_HANDLE(elf_note, elf_section_start(&elf, shdr)),
+ ELF_MAKE_HANDLE(elf_note, elf_section_end(&elf, shdr)));
if ( notes_found )
fprintf(stderr, "using notes from SHT_NOTE section\n");
@@ -292,7 +292,7 @@ int main(int argc, char **argv)
}
shdr = elf_shdr_by_name(&elf, "__xen_guest");
- if (shdr)
+ if (ELF_HANDLE_VALID(shdr))
printf("__xen_guest: %s\n", (char*)elf_section_start(&elf,
shdr));
return 0;
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index 3242f54..566f6f9 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -44,7 +44,7 @@ int elf_xen_parse_features(const char *features,
for ( pos = 0; features[pos] != ''\0''; pos += len )
{
- memset(feature, 0, sizeof(feature));
+ elf_memset_unchecked(feature, 0, sizeof(feature));
for ( len = 0;; len++ )
{
if ( len >= sizeof(feature)-1 )
@@ -96,7 +96,7 @@ int elf_xen_parse_features(const char *features,
int elf_xen_parse_note(struct elf_binary *elf,
struct elf_dom_parms *parms,
- const elf_note *note)
+ ELF_HANDLE_DECL(elf_note) note)
{
/* *INDENT-OFF* */
static const struct {
@@ -215,15 +215,16 @@ int elf_xen_parse_note(struct elf_binary *elf,
static int elf_xen_parse_notes(struct elf_binary *elf,
struct elf_dom_parms *parms,
- const void *start, const void *end)
+ ELF_PTRVAL_CONST_VOID start,
+ ELF_PTRVAL_CONST_VOID end)
{
int xen_elfnotes = 0;
- const elf_note *note;
+ ELF_HANDLE_DECL(elf_note) note;
parms->elf_note_start = start;
parms->elf_note_end = end;
- for ( note = parms->elf_note_start;
- (void *)note < parms->elf_note_end;
+ for ( note = ELF_MAKE_HANDLE(elf_note, parms->elf_note_start);
+ ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
note = elf_note_next(elf, note) )
{
if ( strcmp(elf_note_name(elf, note), "Xen") )
@@ -241,45 +242,46 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
int elf_xen_parse_guest_info(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
- const char *h;
+ ELF_PTRVAL_CONST_CHAR h;
char name[32], value[128];
int len;
h = parms->guest_info;
- while ( *h )
+#define STAR(h) (*(h))
+ while ( STAR(h) )
{
- memset(name, 0, sizeof(name));
- memset(value, 0, sizeof(value));
+ elf_memset_unchecked(name, 0, sizeof(name));
+ elf_memset_unchecked(value, 0, sizeof(value));
for ( len = 0;; len++, h++ )
{
if ( len >= sizeof(name)-1 )
break;
- if ( *h == ''\0'' )
+ if ( STAR(h) == ''\0'' )
break;
- if ( *h == '','' )
+ if ( STAR(h) == '','' )
{
h++;
break;
}
- if ( *h == ''='' )
+ if ( STAR(h) == ''='' )
{
h++;
for ( len = 0;; len++, h++ )
{
if ( len >= sizeof(value)-1 )
break;
- if ( *h == ''\0'' )
+ if ( STAR(h) == ''\0'' )
break;
- if ( *h == '','' )
+ if ( STAR(h) == '','' )
{
h++;
break;
}
- value[len] = *h;
+ value[len] = STAR(h);
}
break;
}
- name[len] = *h;
+ name[len] = STAR(h);
}
elf_msg(elf, "%s: %s=\"%s\"\n", __FUNCTION__, name,
value);
@@ -328,7 +330,8 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
static int elf_xen_note_check(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
- if ( (parms->elf_note_start == NULL) && (parms->guest_info ==
NULL) )
+ if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) &&
+ (ELF_PTRVAL_INVALID(parms->guest_info)) )
{
int machine = elf_uval(elf, elf->ehdr, e_machine);
if ( (machine == EM_386) || (machine == EM_X86_64) )
@@ -457,12 +460,12 @@ static int elf_xen_addr_calc_check(struct elf_binary *elf,
int elf_xen_parse(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
- const elf_shdr *shdr;
- const elf_phdr *phdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
+ ELF_HANDLE_DECL(elf_phdr) phdr;
int xen_elfnotes = 0;
int i, count, rc;
- memset(parms, 0, sizeof(*parms));
+ elf_memset_unchecked(parms, 0, sizeof(*parms));
parms->virt_base = UNSET_ADDR;
parms->virt_entry = UNSET_ADDR;
parms->virt_hypercall = UNSET_ADDR;
@@ -532,11 +535,11 @@ int elf_xen_parse(struct elf_binary *elf,
for ( i = 0; i < count; i++ )
{
shdr = elf_shdr_by_name(elf, "__xen_guest");
- if ( shdr )
+ if ( ELF_HANDLE_VALID(shdr) )
{
parms->guest_info = elf_section_start(elf, shdr);
- parms->elf_note_start = NULL;
- parms->elf_note_end = NULL;
+ parms->elf_note_start = ELF_INVALID_PTRVAL;
+ parms->elf_note_end = ELF_INVALID_PTRVAL;
elf_msg(elf, "%s: __xen_guest: \"%s\"\n",
__FUNCTION__,
parms->guest_info);
elf_xen_parse_guest_info(elf, parms);
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 94257f6..f7fe283 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -26,7 +26,7 @@
int elf_init(struct elf_binary *elf, const char *image, size_t size)
{
- const elf_shdr *shdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
uint64_t i, count, section, offset;
if ( !elf_is_elfbinary(image) )
@@ -35,7 +35,7 @@ int elf_init(struct elf_binary *elf, const char *image, size_t
size)
return -1;
}
- memset(elf, 0, sizeof(*elf));
+ elf_memset_unchecked(elf, 0, sizeof(*elf));
elf->image = image;
elf->size = size;
elf->ehdr = (elf_ehdr *)image;
@@ -65,7 +65,7 @@ int elf_init(struct elf_binary *elf, const char *image, size_t
size)
/* Find section string table. */
section = elf_uval(elf, elf->ehdr, e_shstrndx);
shdr = elf_shdr_by_index(elf, section);
- if ( shdr != NULL )
+ if ( ELF_HANDLE_VALID(shdr) )
elf->sec_strtab = elf_section_start(elf, shdr);
/* Find symbol table and symbol string table. */
@@ -77,9 +77,9 @@ int elf_init(struct elf_binary *elf, const char *image, size_t
size)
continue;
elf->sym_tab = shdr;
shdr = elf_shdr_by_index(elf, elf_uval(elf, shdr, sh_link));
- if ( shdr == NULL )
+ if ( !ELF_HANDLE_VALID(shdr) )
{
- elf->sym_tab = NULL;
+ elf->sym_tab = ELF_INVALID_HANDLE(elf_shdr);
continue;
}
elf->sym_strtab = elf_section_start(elf, shdr);
@@ -113,10 +113,11 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback
*log_callback,
}
static int elf_load_image(struct elf_binary *elf,
- void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
+ ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src,
+ uint64_t filesz, uint64_t memsz)
{
- memcpy(dst, src, filesz);
- memset(dst + filesz, 0, memsz - filesz);
+ elf_memcpy_safe(elf, dst, src, filesz);
+ elf_memset_safe(elf, dst + filesz, 0, memsz - filesz);
return 0;
}
#else
@@ -126,16 +127,17 @@ void elf_set_verbose(struct elf_binary *elf)
elf->verbose = 1;
}
-static int elf_load_image(struct elf_binary *elf,
- void *dst, const void *src, uint64_t filesz, uint64_t
memsz)
+static int elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst,
ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
{
int rc;
if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
return -1;
- rc = raw_copy_to_guest(dst, src, filesz);
+ /* We trust the dom0 kernel image completely, so we don''t care
+ * about overruns etc. here. */
+ rc = raw_copy_to_guest(ELF_UNSAFE_PTR(dst), ELF_UNSAFE_PTR(src), filesz);
if ( rc != 0 )
return -1;
- rc = raw_clear_guest(dst + filesz, memsz - filesz);
+ rc = raw_clear_guest(ELF_UNSAFE_PTR(dst + filesz), memsz - filesz);
if ( rc != 0 )
return -1;
return 0;
@@ -146,10 +148,10 @@ static int elf_load_image(struct elf_binary *elf,
void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart)
{
uint64_t sz;
- const elf_shdr *shdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
int i, type;
- if ( !elf->sym_tab )
+ if ( !ELF_HANDLE_VALID(elf->sym_tab) )
return;
pstart = elf_round_up(elf, pstart);
@@ -166,7 +168,7 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t
pstart)
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
shdr = elf_shdr_by_index(elf, i);
- type = elf_uval(elf, (elf_shdr *)shdr, sh_type);
+ type = elf_uval(elf, shdr, sh_type);
if ( (type == SHT_STRTAB) || (type == SHT_SYMTAB) )
sz = elf_round_up(elf, sz + elf_uval(elf, shdr, sh_size));
}
@@ -177,10 +179,12 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t
pstart)
static void elf_load_bsdsyms(struct elf_binary *elf)
{
- elf_ehdr *sym_ehdr;
+ ELF_HANDLE_DECL_NONCONST(elf_ehdr) sym_ehdr;
unsigned long sz;
- char *maxva, *symbase, *symtab_addr;
- elf_shdr *shdr;
+ ELF_PTRVAL_VOID maxva;
+ ELF_PTRVAL_VOID symbase;
+ ELF_PTRVAL_VOID symtab_addr;
+ ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr;
int i, type;
if ( !elf->bsd_symtab_pstart )
@@ -189,18 +193,18 @@ static void elf_load_bsdsyms(struct elf_binary *elf)
#define elf_hdr_elm(_elf, _hdr, _elm, _val) \
do { \
if ( elf_64bit(_elf) ) \
- (_hdr)->e64._elm = _val; \
+ elf_store_field(_elf, _hdr, e64._elm, _val); \
else \
- (_hdr)->e32._elm = _val; \
+ elf_store_field(_elf, _hdr, e32._elm, _val); \
} while ( 0 )
symbase = elf_get_ptr(elf, elf->bsd_symtab_pstart);
symtab_addr = maxva = symbase + sizeof(uint32_t);
/* Set up Elf header. */
- sym_ehdr = (elf_ehdr *)symtab_addr;
+ sym_ehdr = ELF_MAKE_HANDLE(elf_ehdr, symtab_addr);
sz = elf_uval(elf, elf->ehdr, e_ehsize);
- memcpy(sym_ehdr, elf->ehdr, sz);
+ elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(sym_ehdr),
ELF_HANDLE_PTRVAL(elf->ehdr), sz);
maxva += sz; /* no round up */
elf_hdr_elm(elf, sym_ehdr, e_phoff, 0);
@@ -209,37 +213,39 @@ do { \
elf_hdr_elm(elf, sym_ehdr, e_phnum, 0);
/* Copy Elf section headers. */
- shdr = (elf_shdr *)maxva;
+ shdr = ELF_MAKE_HANDLE(elf_shdr, maxva);
sz = elf_shdr_count(elf) * elf_uval(elf, elf->ehdr, e_shentsize);
- memcpy(shdr, elf->image + elf_uval(elf, elf->ehdr, e_shoff), sz);
- maxva = (char *)(long)elf_round_up(elf, (long)maxva + sz);
+ elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(shdr),
+ ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
+ sz);
+ maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
type = elf_uval(elf, shdr, sh_type);
if ( (type == SHT_STRTAB) || (type == SHT_SYMTAB) )
{
- elf_msg(elf, "%s: shdr %i at 0x%p -> 0x%p\n",
__func__, i,
+ elf_msg(elf, "%s: shdr %i at 0x%"ELF_PRPTRVAL"
-> 0x%"ELF_PRPTRVAL"\n", __func__, i,
elf_section_start(elf, shdr), maxva);
sz = elf_uval(elf, shdr, sh_size);
- memcpy(maxva, elf_section_start(elf, shdr), sz);
+ elf_memcpy_safe(elf, maxva, elf_section_start(elf, shdr), sz);
/* Mangled to be based on ELF header location. */
elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
- maxva = (char *)(long)elf_round_up(elf, (long)maxva + sz);
+ maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva +
sz);
}
- shdr = (elf_shdr *)((long)shdr +
+ shdr = ELF_MAKE_HANDLE(elf_shdr, ELF_HANDLE_PTRVAL(shdr) +
(long)elf_uval(elf, elf->ehdr, e_shentsize));
}
/* Write down the actual sym size. */
- *(uint32_t *)symbase = maxva - symtab_addr;
+ elf_store_val(elf, uint32_t, symbase, maxva - symtab_addr);
#undef elf_ehdr_elm
}
void elf_parse_binary(struct elf_binary *elf)
{
- const elf_phdr *phdr;
+ ELF_HANDLE_DECL(elf_phdr) phdr;
uint64_t low = -1;
uint64_t high = 0;
uint64_t i, count, paddr, memsz;
@@ -267,9 +273,9 @@ void elf_parse_binary(struct elf_binary *elf)
int elf_load_binary(struct elf_binary *elf)
{
- const elf_phdr *phdr;
+ ELF_HANDLE_DECL(elf_phdr) phdr;
uint64_t i, count, paddr, offset, filesz, memsz;
- char *dest;
+ ELF_PTRVAL_VOID dest;
count = elf_uval(elf, elf->ehdr, e_phnum);
for ( i = 0; i < count; i++ )
@@ -282,9 +288,9 @@ int elf_load_binary(struct elf_binary *elf)
filesz = elf_uval(elf, phdr, p_filesz);
memsz = elf_uval(elf, phdr, p_memsz);
dest = elf_get_ptr(elf, paddr);
- elf_msg(elf, "%s: phdr %" PRIu64 " at 0x%p ->
0x%p\n",
- __func__, i, dest, dest + filesz);
- if ( elf_load_image(elf, dest, elf->image + offset, filesz, memsz)
!= 0 )
+ elf_msg(elf, "%s: phdr %" PRIu64 " at
0x%"ELF_PRPTRVAL" -> 0x%"ELF_PRPTRVAL"\n",
+ __func__, i, dest, (ELF_PTRVAL_VOID)(dest + filesz));
+ if ( elf_load_image(elf, dest, ELF_IMAGE_BASE(elf) + offset, filesz,
memsz) != 0 )
return -1;
}
@@ -292,18 +298,18 @@ int elf_load_binary(struct elf_binary *elf)
return 0;
}
-void *elf_get_ptr(struct elf_binary *elf, unsigned long addr)
+ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr)
{
return elf->dest + addr - elf->pstart;
}
uint64_t elf_lookup_addr(struct elf_binary * elf, const char *symbol)
{
- const elf_sym *sym;
+ ELF_HANDLE_DECL(elf_sym) sym;
uint64_t value;
sym = elf_sym_by_name(elf, symbol);
- if ( sym == NULL )
+ if ( !ELF_HANDLE_VALID(sym) )
{
elf_err(elf, "%s: not found: %s\n", __FUNCTION__, symbol);
return -1;
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 1f08407..bf68bcd 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -67,10 +67,10 @@ int elf_phdr_count(struct elf_binary *elf)
return elf_uval(elf, elf->ehdr, e_phnum);
}
-const elf_shdr *elf_shdr_by_name(struct elf_binary *elf, const char *name)
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char
*name)
{
uint64_t count = elf_shdr_count(elf);
- const elf_shdr *shdr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
const char *sname;
int i;
@@ -81,76 +81,80 @@ const elf_shdr *elf_shdr_by_name(struct elf_binary *elf,
const char *name)
if ( sname && !strcmp(sname, name) )
return shdr;
}
- return NULL;
+ return ELF_INVALID_HANDLE(elf_shdr);
}
-const elf_shdr *elf_shdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
{
uint64_t count = elf_shdr_count(elf);
- const void *ptr;
+ ELF_PTRVAL_CONST_VOID ptr;
if ( index >= count )
- return NULL;
+ return ELF_INVALID_HANDLE(elf_shdr);
- ptr = (elf->image
+ ptr = (ELF_IMAGE_BASE(elf)
+ elf_uval(elf, elf->ehdr, e_shoff)
+ elf_uval(elf, elf->ehdr, e_shentsize) * index);
- return ptr;
+ return ELF_MAKE_HANDLE(elf_shdr, ptr);
}
-const elf_phdr *elf_phdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index)
{
uint64_t count = elf_uval(elf, elf->ehdr, e_phnum);
- const void *ptr;
+ ELF_PTRVAL_CONST_VOID ptr;
if ( index >= count )
- return NULL;
+ return ELF_INVALID_HANDLE(elf_phdr);
- ptr = (elf->image
+ ptr = (ELF_IMAGE_BASE(elf)
+ elf_uval(elf, elf->ehdr, e_phoff)
+ elf_uval(elf, elf->ehdr, e_phentsize) * index);
- return ptr;
+ return ELF_MAKE_HANDLE(elf_phdr, ptr);
}
-const char *elf_section_name(struct elf_binary *elf, const elf_shdr * shdr)
+
+const char *elf_section_name(struct elf_binary *elf,
+ ELF_HANDLE_DECL(elf_shdr) shdr)
{
- if ( elf->sec_strtab == NULL )
+ if ( ELF_PTRVAL_INVALID(elf->sec_strtab) )
return "unknown";
+
return elf->sec_strtab + elf_uval(elf, shdr, sh_name);
}
-const void *elf_section_start(struct elf_binary *elf, const elf_shdr * shdr)
+ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
{
- return elf->image + elf_uval(elf, shdr, sh_offset);
+ return ELF_IMAGE_BASE(elf) + elf_uval(elf, shdr, sh_offset);
}
-const void *elf_section_end(struct elf_binary *elf, const elf_shdr * shdr)
+ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
{
- return elf->image
+ return ELF_IMAGE_BASE(elf)
+ elf_uval(elf, shdr, sh_offset) + elf_uval(elf, shdr, sh_size);
}
-const void *elf_segment_start(struct elf_binary *elf, const elf_phdr * phdr)
+ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr)
{
- return elf->image + elf_uval(elf, phdr, p_offset);
+ return ELF_IMAGE_BASE(elf)
+ + elf_uval(elf, phdr, p_offset);
}
-const void *elf_segment_end(struct elf_binary *elf, const elf_phdr * phdr)
+ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr)
{
- return elf->image
+ return ELF_IMAGE_BASE(elf)
+ elf_uval(elf, phdr, p_offset) + elf_uval(elf, phdr, p_filesz);
}
-const elf_sym *elf_sym_by_name(struct elf_binary *elf, const char *symbol)
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol)
{
- const void *ptr = elf_section_start(elf, elf->sym_tab);
- const void *end = elf_section_end(elf, elf->sym_tab);
- const elf_sym *sym;
+ ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
+ ELF_PTRVAL_CONST_VOID end = elf_section_end(elf, elf->sym_tab);
+ ELF_HANDLE_DECL(elf_sym) sym;
uint64_t info, name;
for ( ; ptr < end; ptr += elf_size(elf, sym) )
{
- sym = ptr;
+ sym = ELF_MAKE_HANDLE(elf_sym, ptr);
info = elf_uval(elf, sym, st_info);
name = elf_uval(elf, sym, st_name);
if ( ELF32_ST_BIND(info) != STB_GLOBAL )
@@ -159,33 +163,33 @@ const elf_sym *elf_sym_by_name(struct elf_binary *elf,
const char *symbol)
continue;
return sym;
}
- return NULL;
+ return ELF_INVALID_HANDLE(elf_sym);
}
-const elf_sym *elf_sym_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index)
{
- const void *ptr = elf_section_start(elf, elf->sym_tab);
- const elf_sym *sym;
+ ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
+ ELF_HANDLE_DECL(elf_sym) sym;
- sym = ptr + index * elf_size(elf, sym);
+ sym = ELF_MAKE_HANDLE(elf_sym, ptr + index * elf_size(elf, sym));
return sym;
}
-const char *elf_note_name(struct elf_binary *elf, const elf_note * note)
+const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
- return (void *)note + elf_size(elf, note);
+ return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note);
}
-const void *elf_note_desc(struct elf_binary *elf, const elf_note * note)
+ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
- return (void *)note + elf_size(elf, note) + namesz;
+ return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz;
}
-uint64_t elf_note_numeric(struct elf_binary *elf, const elf_note * note)
+uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
- const void *desc = elf_note_desc(elf, note);
+ ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
int descsz = elf_uval(elf, note, descsz);
switch (descsz)
@@ -200,10 +204,10 @@ uint64_t elf_note_numeric(struct elf_binary *elf, const
elf_note * note)
}
}
-uint64_t elf_note_numeric_array(struct elf_binary *elf, const elf_note *note,
+uint64_t elf_note_numeric_array(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note,
unsigned int unitsz, unsigned int idx)
{
- const void *desc = elf_note_desc(elf, note);
+ ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
int descsz = elf_uval(elf, note, descsz);
if ( descsz % unitsz || idx >= descsz / unitsz )
@@ -220,12 +224,12 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf,
const elf_note *note,
}
}
-const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * note)
+ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
int descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
- return (void *)note + elf_size(elf, note) + namesz + descsz;
+ return ELF_MAKE_HANDLE(elf_note, ELF_HANDLE_PTRVAL(note) + elf_size(elf,
note) + namesz + descsz);
}
/* ------------------------------------------------------------------------ */
@@ -234,10 +238,10 @@ int elf_is_elfbinary(const void *image)
{
const Elf32_Ehdr *ehdr = image;
- return IS_ELF(*ehdr);
+ return IS_ELF(*ehdr); /* fixme unchecked */
}
-int elf_phdr_is_loadable(struct elf_binary *elf, const elf_phdr * phdr)
+int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr)
{
uint64_t p_type = elf_uval(elf, phdr, p_type);
uint64_t p_flags = elf_uval(elf, phdr, p_flags);
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index ae03982..dc9b5ae 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -48,6 +48,97 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
/* ------------------------------------------------------------------------ */
+/* Macros for accessing the input image and output area. */
+
+/*
+ * We abstract away the pointerness of these pointers, replacing
+ * various void*, char* and struct* with the following:
+ * PTRVAL A pointer to a byte; one can do pointer arithmetic
+ * on this.
+ * This replaces variables which were char*,void*
+ * and their const versions, so we provide four
+ * different declaration macros:
+ * ELF_PTRVAL_{,CONST}{VOID,CHAR}
+ * HANDLE A pointer to a struct. There is one of these types
+ * for each pointer type - that is, for each
"structname".
+ * In the arguments to the various HANDLE macros, structname
+ * must be a single identifier which is a typedef.
+ * It is not permitted to do arithmetic on these
+ * pointers. In the current code attempts to do so will
+ * compile, but in the next patch this will become a
+ * compile error.
+ * We provide two declaration macros for const and
+ * non-const pointers.
+ */
+
+#define ELF_REALPTR2PTRVAL(realpointer) (realpointer)
+ /* Converts an actual C pointer into a PTRVAL */
+
+#define ELF_HANDLE_DECL_NONCONST(structname) structname *
+#define ELF_HANDLE_DECL(structname) const structname *
+ /* Provides a type declaration for a HANDLE. */
+ /* May only be used to declare ONE variable at a time */
+
+#define ELF_PTRVAL_VOID void *
+#define ELF_PTRVAL_CHAR char *
+#define ELF_PTRVAL_CONST_VOID const void *
+#define ELF_PTRVAL_CONST_CHAR const char *
+ /* Provides a type declaration for a PTRVAL. */
+ /* May only be used to declare ONE variable at a time */
+
+#define ELF_DEFINE_HANDLE(structname) /* empty */
+ /*
+ * This must be invoked for each HANDLE type to define
+ * the actual C type used for that kind of HANDLE.
+ */
+
+#define ELF_PRPTRVAL "p"
+ /* printf format a la PRId... for a PTRVAL */
+
+#define ELF_MAKE_HANDLE(structname, ptrval) (ptrval)
+ /* Converts a PTRVAL to a HANDLE */
+
+#define ELF_IMAGE_BASE(elf) ((elf)->image)
+ /* Returns the base of the image as a PTRVAL. */
+
+#define ELF_HANDLE_PTRVAL(handleval) ((void*)(handleval))
+ /* Converts a HANDLE to a PTRVAL. */
+
+#define ELF_OBSOLETE_VOIDP_CAST (void*)(uintptr_t)
+ /*
+ * In some places the existing code needs to
+ * - cast away const (the existing code uses const a fair
+ * bit but actually sometimes wants to write to its input)
+ * from a PTRVAL.
+ * - convert an integer representing a pointer to a PTRVAL
+ * This macro provides a suitable cast.
+ */
+
+#define ELF_UNSAFE_PTR(ptrval) ((void*)(ptrval))
+ /*
+ * Turns a PTRVAL into an actual C pointer. Before this is done
+ * the caller must have ensured that the PTRVAL does in fact point
+ * to a permissible location.
+ */
+
+/* PTRVALs can be INVALID (ie, NULL). */
+#define ELF_INVALID_PTRVAL (NULL) /* returns NULL PTRVAL */
+#define ELF_INVALID_HANDLE(structname) /* returns NULL handle */ \
+ ELF_MAKE_HANDLE(structname, ELF_INVALID_PTRVAL)
+#define ELF_PTRVAL_VALID(ptrval) (ptrval) /* } */
+#define ELF_HANDLE_VALID(handleval) (handleval) /* } predicates */
+#define ELF_PTRVAL_INVALID(ptrval) ((ptrval) == NULL) /* } */
+
+/* For internal use by other macros here */
+#define ELF__HANDLE_FIELD_TYPE(handleval, elm) \
+ typeof((handleval)->elm)
+#define ELF__HANDLE_FIELD_OFFSET(handleval, elm) \
+ offsetof(typeof(*(handleval)),elm)
+
+
+/* ------------------------------------------------------------------------ */
+
+
typedef union {
Elf32_Ehdr e32;
Elf64_Ehdr e64;
@@ -83,6 +174,12 @@ typedef union {
Elf64_Note e64;
} elf_note;
+ELF_DEFINE_HANDLE(elf_ehdr)
+ELF_DEFINE_HANDLE(elf_shdr)
+ELF_DEFINE_HANDLE(elf_phdr)
+ELF_DEFINE_HANDLE(elf_sym)
+ELF_DEFINE_HANDLE(elf_note)
+
struct elf_binary {
/* elf binary */
const char *image;
@@ -90,10 +187,10 @@ struct elf_binary {
char class;
char data;
- const elf_ehdr *ehdr;
- const char *sec_strtab;
- const elf_shdr *sym_tab;
- const char *sym_strtab;
+ ELF_HANDLE_DECL(elf_ehdr) ehdr;
+ ELF_PTRVAL_CONST_CHAR sec_strtab;
+ ELF_HANDLE_DECL(elf_shdr) sym_tab;
+ ELF_PTRVAL_CONST_CHAR sym_strtab;
/* loaded to */
char *dest;
@@ -135,45 +232,72 @@ struct elf_binary {
: elf_access_unsigned((elf), (str), \
offsetof(typeof(*(str)),e32.elem), \
sizeof((str)->e32.elem)))
+ /*
+ * Reads an unsigned field in a header structure in the ELF.
+ * str is a HANDLE, and elem is the field name in it.
+ */
#define elf_size(elf, str) \
((ELFCLASS64 == (elf)->class) \
? sizeof((str)->e64) : sizeof((str)->e32))
+ /*
+ * Returns the size of the substructure for the appropriate 32/64-bitness.
+ * str should be a HANDLE.
+ */
-uint64_t elf_access_unsigned(struct elf_binary *elf, const void *ptr,
+uint64_t elf_access_unsigned(struct elf_binary *elf, ELF_PTRVAL_CONST_VOID ptr,
uint64_t offset, size_t size);
+ /* Reads a field at arbitrary offset and alignemnt */
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
+
+#define elf_memcpy_safe(elf, dst, src, sz) memcpy((dst),(src),(sz))
+#define elf_memset_safe(elf, dst, c, sz) memset((dst),(c),(sz))
+ /*
+ * Versions of memcpy and memset which will (in the next patch)
+ * arrange never to write outside permitted areas.
+ */
+
+#define elf_store_val(elf, type, ptr, val) (*(type*)(ptr) = (val))
+ /* Stores a value at a particular PTRVAL. */
+
+#define elf_store_field(elf, hdr, elm, val) \
+ (elf_store_val((elf), ELF__HANDLE_FIELD_TYPE(hdr, elm), \
+ &((hdr)->elm), \
+ (val)))
+ /* Stores a 32/64-bit field. hdr is a HANDLE and elm is the field name. */
+
+
/* ------------------------------------------------------------------------ */
/* xc_libelf_tools.c */
int elf_shdr_count(struct elf_binary *elf);
int elf_phdr_count(struct elf_binary *elf);
-const elf_shdr *elf_shdr_by_name(struct elf_binary *elf, const char *name);
-const elf_shdr *elf_shdr_by_index(struct elf_binary *elf, int index);
-const elf_phdr *elf_phdr_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char
*name);
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index);
-const char *elf_section_name(struct elf_binary *elf, const elf_shdr * shdr);
-const void *elf_section_start(struct elf_binary *elf, const elf_shdr * shdr);
-const void *elf_section_end(struct elf_binary *elf, const elf_shdr * shdr);
+const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr);
+ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
+ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
-const void *elf_segment_start(struct elf_binary *elf, const elf_phdr * phdr);
-const void *elf_segment_end(struct elf_binary *elf, const elf_phdr * phdr);
+ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr);
+ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr);
-const elf_sym *elf_sym_by_name(struct elf_binary *elf, const char *symbol);
-const elf_sym *elf_sym_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol);
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index);
-const char *elf_note_name(struct elf_binary *elf, const elf_note * note);
-const void *elf_note_desc(struct elf_binary *elf, const elf_note * note);
-uint64_t elf_note_numeric(struct elf_binary *elf, const elf_note * note);
-uint64_t elf_note_numeric_array(struct elf_binary *, const elf_note *,
+const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
+ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note);
+uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
+uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
unsigned int unitsz, unsigned int idx);
-const elf_note *elf_note_next(struct elf_binary *elf, const elf_note * note);
+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);
-int elf_phdr_is_loadable(struct elf_binary *elf, const elf_phdr * phdr);
+int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr);
/* ------------------------------------------------------------------------ */
/* xc_libelf_loader.c */
@@ -189,7 +313,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback*,
void elf_parse_binary(struct elf_binary *elf);
int elf_load_binary(struct elf_binary *elf);
-void *elf_get_ptr(struct elf_binary *elf, unsigned long addr);
+ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr);
uint64_t elf_lookup_addr(struct elf_binary *elf, const char *symbol);
void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart); /* private */
@@ -221,9 +345,9 @@ struct xen_elfnote {
struct elf_dom_parms {
/* raw */
- const char *guest_info;
- const void *elf_note_start;
- const void *elf_note_end;
+ ELF_PTRVAL_CONST_CHAR guest_info;
+ ELF_PTRVAL_CONST_VOID elf_note_start;
+ ELF_PTRVAL_CONST_VOID elf_note_end;
struct xen_elfnote elf_notes[XEN_ELFNOTE_MAX + 1];
/* parsed */
@@ -262,10 +386,22 @@ int elf_xen_parse_features(const char *features,
uint32_t *required);
int elf_xen_parse_note(struct elf_binary *elf,
struct elf_dom_parms *parms,
- const elf_note *note);
+ ELF_HANDLE_DECL(elf_note) note);
int elf_xen_parse_guest_info(struct elf_binary *elf,
struct elf_dom_parms *parms);
int elf_xen_parse(struct elf_binary *elf,
struct elf_dom_parms *parms);
+#define elf_memcpy_unchecked memcpy
+#define elf_memset_unchecked memset
+ /*
+ * Unsafe versions of memcpy and memset which take actual C
+ * pointers. These are just like real memcpy and memset.
+ */
+
+
+#define ELF_ADVANCE_DEST(elf, amount) elf->dest += (amount)
+ /* Advances past amount bytes of the current destination area. */
+
+
#endif /* __XEN_LIBELF_H__ */
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 09/21] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
Use the new PTRVAL macros and elf_access_unsigned in
print_l1_mfn_valid_note.
No functional change unless the input is wrong, or we are reading a
file for a different endianness.
Separated out from the previous patch because this change does produce
a difference in the generated code.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
v2: Split out into its own patch.
---
tools/xcutils/readnotes.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 2af047d..7ff2530 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -77,22 +77,23 @@ static void print_numeric_note(const char *prefix, struct
elf_binary *elf,
}
static void print_l1_mfn_valid_note(const char *prefix, struct elf_binary *elf,
- const elf_note *note)
+ ELF_HANDLE_DECL(elf_note) note)
{
int descsz = elf_uval(elf, note, descsz);
- const uint32_t *desc32 = elf_note_desc(elf, note);
- const uint64_t *desc64 = elf_note_desc(elf, note);
+ ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
/* XXX should be able to cope with a list of values. */
switch ( descsz / 2 )
{
case 8:
printf("%s: mask=%#"PRIx64"
value=%#"PRIx64"\n", prefix,
- desc64[0], desc64[1]);
+ elf_access_unsigned(elf, desc, 0, 8),
+ elf_access_unsigned(elf, desc, 8, 8));
break;
case 4:
printf("%s: mask=%#"PRIx32"
value=%#"PRIx32"\n", prefix,
- desc32[0],desc32[1]);
+ (uint32_t)elf_access_unsigned(elf, desc, 0, 4),
+ (uint32_t)elf_access_unsigned(elf, desc, 4, 4));
break;
}
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 10/21] libelf: check nul-terminated strings properly
It is not safe to simply take pointers into the ELF and use them as C
pointers. They might not be properly nul-terminated (and the pointers
might be wild).
So we are going to introduce a new function elf_strval for safely
getting strings. This will check that the addresses are in range and
that there is a proper nul-terminated string. Of course it might
discover that there isn''t. In that case, it will be made to fail.
This means that elf_note_name might fail, too.
For the benefit of call sites which are just going to pass the value
to a printf-like function, we provide elf_strfmt which returns
"(invalid)" on failure rather than NULL.
In this patch we introduce dummy definitions of these functions. We
introduce calls to elf_strval and elf_strfmt everywhere, and update
all the call sites with appropriate error checking.
There is not yet any semantic change, since before this patch all the
places where we introduce elf_strval dereferenced the value anyway, so
it mustn''t have been NULL.
In future patches, when elf_strval is made able return NULL, when it
does so it will mark the elf "broken" so that an appropriate
diagnostic can be printed.
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>
v2: Fix coding style, in one "if" statement.
---
tools/xcutils/readnotes.c | 10 +++++++---
xen/common/libelf/libelf-dominfo.c | 13 ++++++++++---
xen/common/libelf/libelf-tools.c | 10 +++++++---
xen/include/xen/libelf.h | 7 +++++--
4 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 7ff2530..ca86ba5 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -63,7 +63,7 @@ struct setup_header {
static void print_string_note(const char *prefix, struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
- printf("%s: %s\n", prefix, (char*)elf_note_desc(elf, note));
+ printf("%s: %s\n", prefix, elf_strfmt(elf, elf_note_desc(elf,
note)));
}
static void print_numeric_note(const char *prefix, struct elf_binary *elf,
@@ -103,10 +103,13 @@ static int print_notes(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) start,
{
ELF_HANDLE_DECL(elf_note) note;
int notes_found = 0;
+ const char *this_note_name;
for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end); note
= elf_note_next(elf, note) )
{
- if (0 != strcmp(elf_note_name(elf, note), "Xen"))
+ this_note_name = elf_note_name(elf, note);
+ if (NULL == this_note_name ||
+ 0 != strcmp(this_note_name, "Xen"))
continue;
notes_found++;
@@ -294,7 +297,8 @@ int main(int argc, char **argv)
shdr = elf_shdr_by_name(&elf, "__xen_guest");
if (ELF_HANDLE_VALID(shdr))
- printf("__xen_guest: %s\n", (char*)elf_section_start(&elf,
shdr));
+ printf("__xen_guest: %s\n",
+ elf_strfmt(&elf, elf_section_start(&elf,
shdr)));
return 0;
}
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index 566f6f9..ba0dc83 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -137,7 +137,10 @@ int elf_xen_parse_note(struct elf_binary *elf,
if ( note_desc[type].str )
{
- str = elf_note_desc(elf, note);
+ str = elf_strval(elf, elf_note_desc(elf, note));
+ if (str == NULL)
+ /* elf_strval will mark elf broken if it fails so no need to log */
+ return 0;
elf_msg(elf, "%s: %s = \"%s\"\n", __FUNCTION__,
note_desc[type].name, str);
parms->elf_notes[type].type = XEN_ENT_STR;
@@ -220,6 +223,7 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
{
int xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
+ const char *note_name;
parms->elf_note_start = start;
parms->elf_note_end = end;
@@ -227,7 +231,10 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
note = elf_note_next(elf, note) )
{
- if ( strcmp(elf_note_name(elf, note), "Xen") )
+ note_name = elf_note_name(elf, note);
+ if ( note_name == NULL )
+ continue;
+ if ( strcmp(note_name, "Xen") )
continue;
if ( elf_xen_parse_note(elf, parms, note) )
return -1;
@@ -541,7 +548,7 @@ int elf_xen_parse(struct elf_binary *elf,
parms->elf_note_start = ELF_INVALID_PTRVAL;
parms->elf_note_end = ELF_INVALID_PTRVAL;
elf_msg(elf, "%s: __xen_guest: \"%s\"\n",
__FUNCTION__,
- parms->guest_info);
+ elf_strfmt(elf, parms->guest_info));
elf_xen_parse_guest_info(elf, parms);
break;
}
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index bf68bcd..fa7dedd 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -119,7 +119,7 @@ const char *elf_section_name(struct elf_binary *elf,
if ( ELF_PTRVAL_INVALID(elf->sec_strtab) )
return "unknown";
- return elf->sec_strtab + elf_uval(elf, shdr, sh_name);
+ return elf_strval(elf, elf->sec_strtab + elf_uval(elf, shdr, sh_name));
}
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
@@ -151,6 +151,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
ELF_PTRVAL_CONST_VOID end = elf_section_end(elf, elf->sym_tab);
ELF_HANDLE_DECL(elf_sym) sym;
uint64_t info, name;
+ const char *sym_name;
for ( ; ptr < end; ptr += elf_size(elf, sym) )
{
@@ -159,7 +160,10 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
name = elf_uval(elf, sym, st_name);
if ( ELF32_ST_BIND(info) != STB_GLOBAL )
continue;
- if ( strcmp(elf->sym_strtab + name, symbol) )
+ sym_name = elf_strval(elf, elf->sym_strtab + name);
+ if ( sym_name == NULL ) /* out of range, oops */
+ return ELF_INVALID_HANDLE(elf_sym);
+ if ( strcmp(sym_name, symbol) )
continue;
return sym;
}
@@ -177,7 +181,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary
*elf, int index)
const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
- return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note);
+ return elf_strval(elf, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note));
}
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index dc9b5ae..0c2c11e 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -252,6 +252,9 @@ uint64_t elf_access_unsigned(struct elf_binary *elf,
ELF_PTRVAL_CONST_VOID ptr,
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
+#define elf_strval(elf,x) ((const char*)(x)) /* may return NULL in the future
*/
+#define elf_strfmt(elf,x) ((const char*)(x)) /* will return (invalid) instead
*/
+
#define elf_memcpy_safe(elf, dst, src, sz) memcpy((dst),(src),(sz))
#define elf_memset_safe(elf, dst, c, sz) memset((dst),(c),(sz))
/*
@@ -279,7 +282,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index);
ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index);
-const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr);
+const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr); /* might return NULL if inputs are invalid */
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
@@ -289,7 +292,7 @@ ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary
*elf, ELF_HANDLE_DECL(el
ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol);
ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index);
-const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
+const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note); /* may return NULL */
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note);
uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
--
1.7.2.5
We change the ELF_PTRVAL and ELF_HANDLE types and associated macros:
* PTRVAL becomes a uintptr_t, for which we provide a typedef
elf_ptrval. This means no arithmetic done on it can overflow so
the compiler cannot do any malicious invalid pointer arithmetic
"optimisations". It also means that any places where we
dereference one of these pointers without using the appropriate
macros or functions become a compilation error.
So we can be sure that we won''t miss any memory accesses.
All the PTRVAL variables were previously void* or char*, so
the actual address calculations are unchanged.
* ELF_HANDLE becomes a union, one half of which keeps the pointer
value and the other half of which is just there to record the
type.
The new type is not a pointer type so there can be no address
calculations on it whose meaning would change. Every assignment or
access has to go through one of our macros.
* The distinction between const and non-const pointers and char*s
and void*s in libelf goes away. This was not important (and
anyway libelf tended to cast away const in various places).
* The fields elf->image and elf->dest are renamed. That proves
that we haven''t missed any unchecked uses of these actual
pointer values.
* The caller may fill in elf->caller_xdest_base and _size to
specify another range of memory which is safe for libelf to
access, besides the input and output images.
* When accesses fail due to being out of range, we mark the elf
"broken". This will be checked and used for diagnostics in
a following patch.
We do not check for write accesses to the input image. This is
because libelf actually does this in a number of places. So we
simply permit that.
* Each caller of libelf which used to set dest now sets
dest_base and dest_size.
* In xc_dom_load_elf_symtab we provide a new actual-pointer
value hdr_ptr which we get from mapping the guest''s kernel
area and use (checking carefully) as the caller_xdest area.
* The STAR(h) macro in libelf-dominfo.c now uses elf_access_unsigned.
* elf-init uses the new elf_uval_3264 accessor to access the 32-bit
fields, rather than an unchecked field access (ie, unchecked
pointer access).
* elf_uval has been reworked to use elf_uval_3264. Both of these
macros are essentially new in this patch (although they are derived
from the old elf_uval) and need careful review.
* ELF_ADVANCE_DEST is now safe in the sense that you can use it to
chop parts off the front of the dest area but if you chop more than
is available, the dest area is simply set to be empty, preventing
future accesses.
* We introduce some #defines for memcpy, memset, memmove and strcpy:
- We provide elf_memcpy_safe and elf_memset_safe which take
PTRVALs and do checking on the supplied pointers.
- Users inside libelf must all be changed to either
elf_mem*_unchecked (which are just like mem*), or
elf_mem*_safe (which take PTRVALs) and are checked. Any
unchanged call sites become compilation errors.
* We do _not_ at this time fix elf_access_unsigned so that it doesn''t
make unaligned accesses. We hope that unaligned accesses are OK on
every supported architecture. But it does check the supplied
pointer for validity.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: Use allow_size value from xc_dom_vaddr_to_ptr to set xdest_size
correctly.
If ELF_ADVANCE_DEST advances past the end, mark the elf broken.
Always regard NULL allowable region pointers (e.g. dest_base)
as invalid (since NULL pointers don''t point anywhere).
v4: Fix ELF_UNSAFE_PTR to work on 32-bit even when provided 64-bit
values.
Fix xc_dom_load_elf_symtab not to call XC_DOM_PAGE_SIZE
unnecessarily if load is false. This was a regression.
v3.1:
Introduce a change to elf_store_field to undo the effects of
the v3.1 change to the previous patch (the definition there
is not compatible with the new types).
v3: Fix a whitespace error.
v2 was Acked-by: Ian Campbell <ian.campbell@citrix.com>
v2: BUGFIX: elf_strval: Fix loop termination condition to actually work.
BUGFIX: elf_strval: Fix return value to not always be totally wild.
BUGFIX: xc_dom_load_elf_symtab: do proper check for small header size.
xc_dom_load_elf_symtab: narrow scope of `hdr_ptr''.
xc_dom_load_elf_symtab: split out uninit''d symtab.class ref fix.
More comments on the lifetime/validity of elf-> dest ptrs etc.
libelf.h: write "obsolete" out in full
libelf.h: rename "dontuse" to "typeonly" and add doc
comment
elf_ptrval_in_range: Document trustedness of arguments.
Style and commit message fixes.
---
tools/libxc/xc_dom_elfloader.c | 49 ++++++++--
tools/libxc/xc_hvm_build_x86.c | 10 +-
xen/arch/x86/domain_build.c | 3 +-
xen/common/libelf/libelf-dominfo.c | 2 +-
xen/common/libelf/libelf-loader.c | 16 ++--
xen/common/libelf/libelf-private.h | 13 +++
xen/common/libelf/libelf-tools.c | 106 ++++++++++++++++++-
xen/include/xen/libelf.h | 199 +++++++++++++++++++++++++-----------
8 files changed, 312 insertions(+), 86 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index b8089bc..c038d1c 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -128,20 +128,30 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
if ( load )
{
- size_t allow_size; /* will be used in a forthcoming XSA-55 patch */
+ char *hdr_ptr;
+ size_t allow_size;
+
if ( !dom->bsd_symtab_start )
return 0;
size = dom->kernel_seg.vend - dom->bsd_symtab_start;
- hdr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start,
&allow_size);
- *(int *)hdr = size - sizeof(int);
+ hdr_ptr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start,
&allow_size);
+ elf->caller_xdest_base = hdr_ptr;
+ elf->caller_xdest_size = allow_size;
+ hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
+ elf_store_val(elf, int, hdr, size - sizeof(int));
}
else
{
+ char *hdr_ptr;
+
size = sizeof(int) + elf_size(elf, elf->ehdr) +
elf_shdr_count(elf) * elf_size(elf, shdr);
- hdr = xc_dom_malloc(dom, size);
- if ( hdr == NULL )
+ hdr_ptr = xc_dom_malloc(dom, size);
+ if ( hdr_ptr == NULL )
return 0;
+ elf->caller_xdest_base = hdr_ptr;
+ elf->caller_xdest_size = size;
+ hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
}
@@ -169,9 +179,32 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
ehdr->e_shoff = elf_size(elf, elf->ehdr);
ehdr->e_shstrndx = SHN_UNDEF;
}
- if ( elf_init(&syms, hdr + sizeof(int), size - sizeof(int)) )
+ if ( elf->caller_xdest_size < sizeof(int) )
+ {
+ DOMPRINTF("%s/%s: header size %"PRIx64" too small",
+ __FUNCTION__, load ? "load" : "parse",
+ (uint64_t)elf->caller_xdest_size);
+ return -1;
+ }
+ if ( elf_init(&syms, elf->caller_xdest_base + sizeof(int),
+ elf->caller_xdest_size - sizeof(int)) )
return -1;
+ /*
+ * The caller_xdest_{base,size} and dest_{base,size} need to
+ * remain valid so long as each struct elf_image does. The
+ * principle we adopt is that these values are set when the
+ * memory is allocated or mapped, and cleared when (and if)
+ * they are unmapped.
+ *
+ * Mappings of the guest are normally undone by xc_dom_unmap_all
+ * (directly or via xc_dom_release). We do not explicitly clear
+ * these because in fact that happens only at the end of
+ * xc_dom_boot_image, at which time all of these ELF loading
+ * functions have returned. No relevant struct elf_binary*
+ * escapes this file.
+ */
+
xc_elf_set_logfile(dom->xch, &syms, 1);
symtab = dom->bsd_symtab_start + sizeof(int);
@@ -310,8 +343,10 @@ static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
{
struct elf_binary *elf = dom->private_loader;
int rc;
+ xen_pfn_t pages;
- elf->dest = xc_dom_seg_to_ptr(dom, &dom->kernel_seg);
+ elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg,
&pages);
+ elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom);
rc = elf_load_binary(elf);
if ( rc < 0 )
{
diff --git a/tools/libxc/xc_hvm_build_x86.c b/tools/libxc/xc_hvm_build_x86.c
index 39f93a3..eff55a4 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -137,11 +137,12 @@ static int loadelfimage(xc_interface *xch, struct
elf_binary *elf,
for ( i = 0; i < pages; i++ )
entries[i].mfn = parray[(elf->pstart >> PAGE_SHIFT) + i];
- elf->dest = xc_map_foreign_ranges(
+ elf->dest_base = xc_map_foreign_ranges(
xch, dom, pages << PAGE_SHIFT, PROT_READ | PROT_WRITE, 1 <<
PAGE_SHIFT,
entries, pages);
- if ( elf->dest == NULL )
+ if ( elf->dest_base == NULL )
goto err;
+ elf->dest_size = pages * PAGE_SIZE;
ELF_ADVANCE_DEST(elf, elf->pstart & (PAGE_SIZE - 1));
@@ -150,8 +151,9 @@ static int loadelfimage(xc_interface *xch, struct elf_binary
*elf,
if ( rc < 0 )
PERROR("Failed to load elf binary\n");
- munmap(elf->dest, pages << PAGE_SHIFT);
- elf->dest = NULL;
+ munmap(elf->dest_base, pages << PAGE_SHIFT);
+ elf->dest_base = NULL;
+ elf->dest_size = 0;
err:
free(entries);
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 9980ea2..db31a91 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -765,7 +765,8 @@ int __init construct_dom0(
mapcache_override_current(v);
/* Copy the OS image and free temporary buffer. */
- elf.dest = (void*)vkern_start;
+ elf.dest_base = (void*)vkern_start;
+ elf.dest_size = vkern_end - vkern_start;
rc = elf_load_binary(&elf);
if ( rc < 0 )
{
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index ba0dc83..b9a4e25 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -254,7 +254,7 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
int len;
h = parms->guest_info;
-#define STAR(h) (*(h))
+#define STAR(h) (elf_access_unsigned(elf, (h), 0, 1))
while ( STAR(h) )
{
elf_memset_unchecked(name, 0, sizeof(name));
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index f7fe283..878552e 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -24,23 +24,25 @@
/* ------------------------------------------------------------------------ */
-int elf_init(struct elf_binary *elf, const char *image, size_t size)
+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) )
+ if ( !elf_is_elfbinary(image_input) )
{
elf_err(elf, "%s: not an ELF binary\n", __FUNCTION__);
return -1;
}
elf_memset_unchecked(elf, 0, sizeof(*elf));
- elf->image = image;
+ elf->image_base = image_input;
elf->size = size;
- elf->ehdr = (elf_ehdr *)image;
- elf->class = elf->ehdr->e32.e_ident[EI_CLASS];
- elf->data = elf->ehdr->e32.e_ident[EI_DATA];
+ elf->ehdr = ELF_MAKE_HANDLE(elf_ehdr, (elf_ptrval)image_input);
+ elf->class = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_CLASS]);
+ elf->data = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_DATA]);
+ elf->caller_xdest_base = NULL;
+ elf->caller_xdest_size = 0;
/* Sanity check phdr. */
offset = elf_uval(elf, elf->ehdr, e_phoff) +
@@ -300,7 +302,7 @@ int elf_load_binary(struct elf_binary *elf)
ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr)
{
- return elf->dest + addr - elf->pstart;
+ return ELF_REALPTR2PTRVAL(elf->dest_base) + addr - elf->pstart;
}
uint64_t elf_lookup_addr(struct elf_binary * elf, const char *symbol)
diff --git a/xen/common/libelf/libelf-private.h
b/xen/common/libelf/libelf-private.h
index 0d4dcf6..0bd9e66 100644
--- a/xen/common/libelf/libelf-private.h
+++ b/xen/common/libelf/libelf-private.h
@@ -86,6 +86,19 @@ do { strncpy((d),(s),sizeof((d))-1); \
#endif
+#undef memcpy
+#undef memset
+#undef memmove
+#undef strcpy
+
+#define memcpy MISTAKE_unspecified_memcpy
+#define memset MISTAKE_unspecified_memset
+#define memmove MISTAKE_unspecified_memmove
+#define strcpy MISTAKE_unspecified_strcpy
+ /* This prevents libelf from using these undecorated versions
+ * of memcpy, memset, memmove and strcpy. Every call site
+ * must either use elf_mem*_unchecked, or elf_mem*_safe. */
+
#endif /* __LIBELF_PRIVATE_H_ */
/*
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index fa7dedd..08ab027 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -20,28 +20,100 @@
/* ------------------------------------------------------------------------ */
-uint64_t elf_access_unsigned(struct elf_binary * elf, const void *ptr,
- uint64_t offset, size_t size)
+void elf_mark_broken(struct elf_binary *elf, const char *msg)
{
+ if ( elf->broken == NULL )
+ elf->broken = msg;
+}
+
+const char *elf_check_broken(const struct elf_binary *elf)
+{
+ return elf->broken;
+}
+
+static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size,
+ const void *region, uint64_t regionsize)
+ /*
+ * Returns true if the putative memory area [ptrval,ptrval+size>
+ * is completely inside the region [region,region+regionsize>.
+ *
+ * ptrval and size are the untrusted inputs to be checked.
+ * region and regionsize are trusted and must be correct and valid,
+ * although it is OK for region to perhaps be maliciously NULL
+ * (but not some other malicious value).
+ */
+{
+ elf_ptrval regionp = (elf_ptrval)region;
+
+ if ( (region == NULL) ||
+ (ptrval < regionp) || /* start is before region */
+ (ptrval > regionp + regionsize) || /* start is after region */
+ (size > regionsize - (ptrval - regionp)) ) /* too big */
+ return 0;
+ return 1;
+}
+
+int elf_access_ok(struct elf_binary * elf,
+ uint64_t ptrval, size_t size)
+{
+ if ( elf_ptrval_in_range(ptrval, size, elf->image_base, elf->size) )
+ return 1;
+ if ( elf_ptrval_in_range(ptrval, size, elf->dest_base,
elf->dest_size) )
+ return 1;
+ if ( elf_ptrval_in_range(ptrval, size,
+ elf->caller_xdest_base,
elf->caller_xdest_size) )
+ return 1;
+ elf_mark_broken(elf, "out of range access");
+ return 0;
+}
+
+void elf_memcpy_safe(struct elf_binary *elf, elf_ptrval dst,
+ elf_ptrval src, size_t size)
+{
+ if ( elf_access_ok(elf, dst, size) &&
+ elf_access_ok(elf, src, size) )
+ {
+ /* use memmove because these checks do not prove that the
+ * regions don''t overlap and overlapping regions grant
+ * permission for compiler malice */
+ elf_memmove_unchecked(ELF_UNSAFE_PTR(dst), ELF_UNSAFE_PTR(src), size);
+ }
+}
+
+void elf_memset_safe(struct elf_binary *elf, elf_ptrval dst, int c, size_t
size)
+{
+ if ( elf_access_ok(elf, dst, size) )
+ {
+ elf_memset_unchecked(ELF_UNSAFE_PTR(dst), c, size);
+ }
+}
+
+uint64_t elf_access_unsigned(struct elf_binary * elf, elf_ptrval base,
+ uint64_t moreoffset, size_t size)
+{
+ elf_ptrval ptrval = base + moreoffset;
int need_swap = elf_swap(elf);
const uint8_t *u8;
const uint16_t *u16;
const uint32_t *u32;
const uint64_t *u64;
+ if ( !elf_access_ok(elf, ptrval, size) )
+ return 0;
+
switch ( size )
{
case 1:
- u8 = ptr + offset;
+ u8 = (const void*)ptrval;
return *u8;
case 2:
- u16 = ptr + offset;
+ u16 = (const void*)ptrval;
return need_swap ? bswap_16(*u16) : *u16;
case 4:
- u32 = ptr + offset;
+ u32 = (const void*)ptrval;
return need_swap ? bswap_32(*u32) : *u32;
case 8:
- u64 = ptr + offset;
+ u64 = (const void*)ptrval;
return need_swap ? bswap_64(*u64) : *u64;
default:
return 0;
@@ -122,6 +194,28 @@ const char *elf_section_name(struct elf_binary *elf,
return elf_strval(elf, elf->sec_strtab + elf_uval(elf, shdr, sh_name));
}
+const char *elf_strval(struct elf_binary *elf, elf_ptrval start)
+{
+ uint64_t length;
+
+ for ( length = 0; ; length++ ) {
+ if ( !elf_access_ok(elf, start + length, 1) )
+ return NULL;
+ if ( !elf_access_unsigned(elf, start, length, 1) )
+ /* ok */
+ return ELF_UNSAFE_PTR(start);
+ }
+}
+
+const char *elf_strfmt(struct elf_binary *elf, elf_ptrval start)
+{
+ const char *str = elf_strval(elf, start);
+
+ if ( str == NULL )
+ return "(invalid)";
+ return str;
+}
+
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
{
return ELF_IMAGE_BASE(elf) + elf_uval(elf, shdr, sh_offset);
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 0c2c11e..783f125 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -57,8 +57,9 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
* on this.
* This replaces variables which were char*,void*
* and their const versions, so we provide four
- * different declaration macros:
+ * different obsolete declaration macros:
* ELF_PTRVAL_{,CONST}{VOID,CHAR}
+ * New code can simply use the elf_ptrval typedef.
* HANDLE A pointer to a struct. There is one of these types
* for each pointer type - that is, for each
"structname".
* In the arguments to the various HANDLE macros, structname
@@ -67,54 +68,66 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
* pointers. In the current code attempts to do so will
* compile, but in the next patch this will become a
* compile error.
- * We provide two declaration macros for const and
- * non-const pointers.
+ * We also provide a second declaration macro for
+ * pointers which were to const; this is obsolete.
*/
-#define ELF_REALPTR2PTRVAL(realpointer) (realpointer)
+typedef uintptr_t elf_ptrval;
+
+#define ELF_REALPTR2PTRVAL(realpointer) ((elf_ptrval)(realpointer))
/* Converts an actual C pointer into a PTRVAL */
-#define ELF_HANDLE_DECL_NONCONST(structname) structname *
-#define ELF_HANDLE_DECL(structname) const structname *
+#define ELF_HANDLE_DECL_NONCONST(structname) structname##_handle /*obsolete*/
+#define ELF_HANDLE_DECL(structname) structname##_handle
/* Provides a type declaration for a HANDLE. */
- /* May only be used to declare ONE variable at a time */
-#define ELF_PTRVAL_VOID void *
-#define ELF_PTRVAL_CHAR char *
-#define ELF_PTRVAL_CONST_VOID const void *
-#define ELF_PTRVAL_CONST_CHAR const char *
- /* Provides a type declaration for a PTRVAL. */
- /* May only be used to declare ONE variable at a time */
+#define ELF_PTRVAL_VOID elf_ptrval /*obsolete*/
+#define ELF_PTRVAL_CHAR elf_ptrval /*obsolete*/
+#define ELF_PTRVAL_CONST_VOID elf_ptrval /*obsolete*/
+#define ELF_PTRVAL_CONST_CHAR elf_ptrval /*obsolete*/
+
+#ifdef __XEN__
+# define ELF_PRPTRVAL "lu"
+ /*
+ * PRIuPTR is misdefined in xen/include/xen/inttypes.h, on 32-bit,
+ * to "u", when in fact uintptr_t is an unsigned long.
+ */
+#else
+# define ELF_PRPTRVAL PRIuPTR
+#endif
+ /* printf format a la PRId... for a PTRVAL */
-#define ELF_DEFINE_HANDLE(structname) /* empty */
+#define ELF_DEFINE_HANDLE(structname) \
+ typedef union { \
+ elf_ptrval ptrval; \
+ const structname *typeonly; /* for sizeof, offsetof, &c only */ \
+ } structname##_handle;
/*
* This must be invoked for each HANDLE type to define
* the actual C type used for that kind of HANDLE.
*/
-#define ELF_PRPTRVAL "p"
- /* printf format a la PRId... for a PTRVAL */
-
-#define ELF_MAKE_HANDLE(structname, ptrval) (ptrval)
+#define ELF_MAKE_HANDLE(structname, ptrval) ((structname##_handle){ ptrval
})
/* Converts a PTRVAL to a HANDLE */
-#define ELF_IMAGE_BASE(elf) ((elf)->image)
+#define ELF_IMAGE_BASE(elf) ((elf_ptrval)(elf)->image_base)
/* Returns the base of the image as a PTRVAL. */
-#define ELF_HANDLE_PTRVAL(handleval) ((void*)(handleval))
+#define ELF_HANDLE_PTRVAL(handleval) ((handleval).ptrval)
/* Converts a HANDLE to a PTRVAL. */
-#define ELF_OBSOLETE_VOIDP_CAST (void*)(uintptr_t)
+#define ELF_OBSOLETE_VOIDP_CAST /*empty*/
/*
- * In some places the existing code needs to
+ * In some places the old code used to need to
* - cast away const (the existing code uses const a fair
* bit but actually sometimes wants to write to its input)
* from a PTRVAL.
* - convert an integer representing a pointer to a PTRVAL
- * This macro provides a suitable cast.
+ * Nowadays all of these re uintptr_ts so there is no const problem
+ * and no need for any casting.
*/
-#define ELF_UNSAFE_PTR(ptrval) ((void*)(ptrval))
+#define ELF_UNSAFE_PTR(ptrval) ((void*)(elf_ptrval)(ptrval))
/*
* Turns a PTRVAL into an actual C pointer. Before this is done
* the caller must have ensured that the PTRVAL does in fact point
@@ -122,23 +135,25 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
*/
/* PTRVALs can be INVALID (ie, NULL). */
-#define ELF_INVALID_PTRVAL (NULL) /* returns NULL PTRVAL */
+#define ELF_INVALID_PTRVAL ((elf_ptrval)0) /* returns NULL PTRVAL */
#define ELF_INVALID_HANDLE(structname) /* returns NULL handle */ \
ELF_MAKE_HANDLE(structname, ELF_INVALID_PTRVAL)
-#define ELF_PTRVAL_VALID(ptrval) (ptrval) /* } */
-#define ELF_HANDLE_VALID(handleval) (handleval) /* } predicates */
-#define ELF_PTRVAL_INVALID(ptrval) ((ptrval) == NULL) /* } */
+#define ELF_PTRVAL_VALID(ptrval) (!!(ptrval)) /* } */
+#define ELF_HANDLE_VALID(handleval) (!!(handleval).ptrval) /* } predicates */
+#define ELF_PTRVAL_INVALID(ptrval) (!ELF_PTRVAL_VALID((ptrval))) /* } */
+
+#define ELF_MAX_PTRVAL (~(elf_ptrval)0)
+ /* PTRVAL value guaranteed to compare > to any valid PTRVAL */
/* For internal use by other macros here */
#define ELF__HANDLE_FIELD_TYPE(handleval, elm) \
- typeof((handleval)->elm)
+ typeof((handleval).typeonly->elm)
#define ELF__HANDLE_FIELD_OFFSET(handleval, elm) \
- offsetof(typeof(*(handleval)),elm)
+ offsetof(typeof(*(handleval).typeonly),elm)
/* ------------------------------------------------------------------------ */
-
typedef union {
Elf32_Ehdr e32;
Elf64_Ehdr e64;
@@ -182,7 +197,7 @@ ELF_DEFINE_HANDLE(elf_note)
struct elf_binary {
/* elf binary */
- const char *image;
+ const void *image_base;
size_t size;
char class;
char data;
@@ -190,10 +205,16 @@ struct elf_binary {
ELF_HANDLE_DECL(elf_ehdr) ehdr;
ELF_PTRVAL_CONST_CHAR sec_strtab;
ELF_HANDLE_DECL(elf_shdr) sym_tab;
- ELF_PTRVAL_CONST_CHAR sym_strtab;
+ uint64_t sym_strtab;
/* loaded to */
- char *dest;
+ /*
+ * dest_base and dest_size are trusted and must be correct;
+ * whenever dest_size is not 0, both of these must be valid
+ * so long as the struct elf_binary is in use.
+ */
+ char *dest_base;
+ size_t dest_size;
uint64_t pstart;
uint64_t pend;
uint64_t reloc_offset;
@@ -201,12 +222,22 @@ struct elf_binary {
uint64_t bsd_symtab_pstart;
uint64_t bsd_symtab_pend;
+ /*
+ * caller''s other acceptable destination
+ *
+ * Again, these are trusted and must be valid (or 0) so long
+ * as the struct elf_binary is in use.
+ */
+ void *caller_xdest_base;
+ uint64_t caller_xdest_size;
+
#ifndef __XEN__
/* misc */
elf_log_callback *log_callback;
void *log_caller_data;
#endif
int verbose;
+ const char *broken;
};
/* ------------------------------------------------------------------------ */
@@ -224,22 +255,27 @@ struct elf_binary {
#define elf_lsb(elf) (ELFDATA2LSB == (elf)->data)
#define elf_swap(elf) (NATIVE_ELFDATA != (elf)->data)
-#define elf_uval(elf, str, elem) \
- ((ELFCLASS64 == (elf)->class) \
- ? elf_access_unsigned((elf), (str), \
- offsetof(typeof(*(str)),e64.elem), \
- sizeof((str)->e64.elem)) \
- : elf_access_unsigned((elf), (str), \
- offsetof(typeof(*(str)),e32.elem), \
- sizeof((str)->e32.elem)))
+#define elf_uval_3264(elf, handle, elem) \
+ elf_access_unsigned((elf), (handle).ptrval, \
+ offsetof(typeof(*(handle).typeonly),elem), \
+ sizeof((handle).typeonly->elem))
+
+#define elf_uval(elf, handle, elem) \
+ ((ELFCLASS64 == (elf)->class) \
+ ? elf_uval_3264(elf, handle, e64.elem) \
+ : elf_uval_3264(elf, handle, e32.elem))
/*
* Reads an unsigned field in a header structure in the ELF.
* str is a HANDLE, and elem is the field name in it.
*/
-#define elf_size(elf, str) \
+
+#define elf_size(elf, handle_or_handletype) ({ \
+ typeof(handle_or_handletype) elf_size__dummy; \
((ELFCLASS64 == (elf)->class) \
- ? sizeof((str)->e64) : sizeof((str)->e32))
+ ? sizeof(elf_size__dummy.typeonly->e64) \
+ : sizeof(elf_size__dummy.typeonly->e32)); \
+})
/*
* Returns the size of the substructure for the appropriate 32/64-bitness.
* str should be a HANDLE.
@@ -251,23 +287,37 @@ uint64_t elf_access_unsigned(struct elf_binary *elf,
ELF_PTRVAL_CONST_VOID ptr,
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
+const char *elf_strval(struct elf_binary *elf, elf_ptrval start);
+ /* may return NULL if the string is out of range etc. */
-#define elf_strval(elf,x) ((const char*)(x)) /* may return NULL in the future
*/
-#define elf_strfmt(elf,x) ((const char*)(x)) /* will return (invalid) instead
*/
+const char *elf_strfmt(struct elf_binary *elf, elf_ptrval start);
+ /* like elf_strval but returns "(invalid)" instead of NULL */
-#define elf_memcpy_safe(elf, dst, src, sz) memcpy((dst),(src),(sz))
-#define elf_memset_safe(elf, dst, c, sz) memset((dst),(c),(sz))
+void elf_memcpy_safe(struct elf_binary*, elf_ptrval dst, elf_ptrval src,
size_t);
+void elf_memset_safe(struct elf_binary*, elf_ptrval dst, int c, size_t);
/*
- * Versions of memcpy and memset which will (in the next patch)
- * arrange never to write outside permitted areas.
+ * Versions of memcpy and memset which arrange never to write
+ * outside permitted areas.
*/
-#define elf_store_val(elf, type, ptr, val) (*(type*)(ptr) = (val))
+int elf_access_ok(struct elf_binary * elf,
+ uint64_t ptrval, size_t size);
+
+#define elf_store_val(elf, type, ptr, val) \
+ ({ \
+ typeof(type) elf_store__val = (val); \
+ elf_ptrval elf_store__targ = ptr; \
+ if (elf_access_ok((elf), elf_store__targ, \
+ sizeof(elf_store__val))) { \
+ elf_memcpy_unchecked((void*)elf_store__targ, &elf_store__val, \
+ sizeof(elf_store__val)); \
+ } \
+ }) \
/* Stores a value at a particular PTRVAL. */
-#define elf_store_field(elf, hdr, elm, val) \
- (elf_store_val((elf), ELF__HANDLE_FIELD_TYPE(hdr, elm), \
- &((hdr)->elm), \
+#define elf_store_field(elf, hdr, elm, val) \
+ (elf_store_val((elf), ELF__HANDLE_FIELD_TYPE(hdr, elm), \
+ ELF_HANDLE_PTRVAL(hdr) + ELF__HANDLE_FIELD_OFFSET(hdr, elm),
\
(val)))
/* Stores a 32/64-bit field. hdr is a HANDLE and elm is the field name. */
@@ -306,6 +356,10 @@ int elf_phdr_is_loadable(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr)
/* xc_libelf_loader.c */
int elf_init(struct elf_binary *elf, const char *image, size_t size);
+ /*
+ * image and size must be correct. They will be recorded in
+ * *elf, and must remain valid while the elf is in use.
+ */
#ifdef __XEN__
void elf_set_verbose(struct elf_binary *elf);
#else
@@ -321,6 +375,9 @@ uint64_t elf_lookup_addr(struct elf_binary *elf, const char
*symbol);
void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart); /* private */
+void elf_mark_broken(struct elf_binary *elf, const char *msg);
+const char *elf_check_broken(const struct elf_binary *elf); /* NULL means OK */
+
/* ------------------------------------------------------------------------ */
/* xc_libelf_relocate.c */
@@ -395,16 +452,38 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
int elf_xen_parse(struct elf_binary *elf,
struct elf_dom_parms *parms);
-#define elf_memcpy_unchecked memcpy
-#define elf_memset_unchecked memset
+static inline void *elf_memcpy_unchecked(void *dest, const void *src, size_t n)
+ { return memcpy(dest, src, n); }
+static inline void *elf_memmove_unchecked(void *dest, const void *src, size_t
n)
+ { return memmove(dest, src, n); }
+static inline void *elf_memset_unchecked(void *s, int c, size_t n)
+ { return memset(s, c, n); }
/*
- * Unsafe versions of memcpy and memset which take actual C
- * pointers. These are just like real memcpy and memset.
+ * Unsafe versions of memcpy, memmove memset which take actual C
+ * pointers. These are just like the real functions.
+ * We provide these so that in libelf-private.h we can #define
+ * memcpy, memset and memmove to undefined MISTAKE things.
*/
-#define ELF_ADVANCE_DEST(elf, amount) elf->dest += (amount)
- /* Advances past amount bytes of the current destination area. */
+/* Advances past amount bytes of the current destination area. */
+static inline void ELF_ADVANCE_DEST(struct elf_binary *elf, uint64_t amount)
+{
+ if ( elf->dest_base == NULL )
+ {
+ elf_mark_broken(elf, "advancing in null image");
+ }
+ else if ( elf->dest_size >= amount )
+ {
+ elf->dest_base += amount;
+ elf->dest_size -= amount;
+ }
+ else
+ {
+ elf->dest_size = 0;
+ elf_mark_broken(elf, "advancing past end (image very
short?)");
+ }
+}
#endif /* __XEN_LIBELF_H__ */
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 12/21] 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.
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>
v2: Style fix.
Fix commit message subject.
---
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 | 2 +-
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index c038d1c..f14b053 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -93,7 +93,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 c5519d8..58fda16 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 878552e..6c43c34 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 08ab027..b613593 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 783f125..c54c90b 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -349,7 +349,7 @@ 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);
+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
Ian Jackson
2013-Jun-06 18:51 UTC
[PATCH 13/21] 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".
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
v5: Fix two whitespace errors.
v3.1:
Add error check to xc_dom_parse_elf_kernel.
Move check in xc_hvm_build_x86.c:setup_guest to right place.
v2 was Acked-by: Ian Campbell <ian.campbell@citrix.com>
v2 was Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
v2: Style fixes.
---
tools/libxc/xc_dom_elfloader.c | 25 +++++++++++++++++++++----
tools/libxc/xc_hvm_build_x86.c | 3 +++
tools/xcutils/readnotes.c | 3 +++
xen/arch/arm/kernel.c | 10 ++++++++++
xen/arch/x86/domain_build.c | 28 +++++++++++++++++++++-------
5 files changed, 58 insertions(+), 11 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index f14b053..a0d39b3 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -274,6 +274,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__);
@@ -310,19 +317,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 */
@@ -336,7 +347,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 eff55a4..8bb0178 100644
--- a/tools/libxc/xc_hvm_build_x86.c
+++ b/tools/libxc/xc_hvm_build_x86.c
@@ -524,6 +524,9 @@ static int setup_guest(xc_interface *xch,
error_out:
rc = -1;
out:
+ if ( elf_check_broken(&elf) )
+ ERROR("HVM ELF broken: %s", elf_check_broken(&elf));
+
/* ensure no unclaimed pages are left unused */
xc_domain_claim_pages(xch, dom, 0 /* cancels the claim */);
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index ca86ba5..4394905 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -300,6 +300,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 8f4a60d..43cf2ab 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -171,6 +171,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(size);
info->kernel_img = alloc_xenheap_pages(info->kernel_order, 0);
if ( info->kernel_img == NULL )
@@ -194,8 +196,16 @@ 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));
+
free_xenheap_pages(info->kernel_img, info->kernel_order);
return rc;
}
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index db31a91..03fe845 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -380,7 +380,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;
@@ -408,14 +408,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 ( compat32 )
@@ -596,7 +598,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) )
@@ -771,7 +774,7 @@ int __init construct_dom0(
if ( rc < 0 )
{
printk("Failed to load the kernel binary\n");
- return rc;
+ goto out;
}
bootstrap_map(NULL);
@@ -783,7 +786,8 @@ int __init construct_dom0(
mapcache_override_current(NULL);
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);
@@ -1133,9 +1137,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
We want to remove uses of "int" because signed integers have
undesirable undefined behaviours on overflow. Malicious compilers can
turn apparently-correct code into code with security vulnerabilities
etc.
In this patch we change all the booleans in libelf to C99 bool,
from <stdbool.h>.
For the one visible libelf boolean in libxc''s public interface we
retain the use of int to avoid changing the ABI; libxc converts it to
a bool for consumption by libelf.
It is OK to change all values only ever used as booleans to _Bool
(bool) because conversion from any scalar type to a _Bool works the
same as the boolean test in if() or ?: and is always defined (C99
6.3.1.2). But we do need to check that all these variables really are
only ever used that way. (It is theoretically possible that the old
code truncated some 64-bit values to 32-bit ints which might become
zero depending on the value, which would mean a behavioural change in
this patch, but it seems implausible that treating 0x????????00000000
as false could have been intended.)
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
v3: Use <stdbool.h>''s bool (or _Bool) instead of defining
elf_bool.
Split this into a separate patch.
---
tools/libxc/xc_dom_elfloader.c | 8 ++++----
xen/common/libelf/libelf-dominfo.c | 2 +-
xen/common/libelf/libelf-loader.c | 4 ++--
xen/common/libelf/libelf-private.h | 2 +-
xen/common/libelf/libelf-tools.c | 10 +++++-----
xen/include/xen/libelf.h | 18 ++++++++++--------
6 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index a0d39b3..8f9c2fb 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -34,7 +34,7 @@
/* ------------------------------------------------------------------------ */
static void log_callback(struct elf_binary *elf, void *caller_data,
- int iserr, const char *fmt, va_list al) {
+ bool iserr, const char *fmt, va_list al) {
xc_interface *xch = caller_data;
xc_reportv(xch,
@@ -46,7 +46,7 @@ static void log_callback(struct elf_binary *elf, void
*caller_data,
void xc_elf_set_logfile(xc_interface *xch, struct elf_binary *elf,
int verbose) {
- elf_set_log(elf, log_callback, xch, verbose);
+ elf_set_log(elf, log_callback, xch, verbose /* convert to bool */);
}
/* ------------------------------------------------------------------------ */
@@ -82,7 +82,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
/* ------------------------------------------------------------------------ */
/* parse elf binary */
-static int check_elf_kernel(struct xc_dom_image *dom, int verbose)
+static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
{
if ( dom->kernel_blob == NULL )
{
@@ -110,7 +110,7 @@ static int xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
}
static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
- struct elf_binary *elf, int load)
+ struct elf_binary *elf, bool load)
{
struct elf_binary syms;
ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr; ELF_HANDLE_DECL(elf_shdr) shdr2;
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index b9a4e25..c4ced67 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -101,7 +101,7 @@ int elf_xen_parse_note(struct elf_binary *elf,
/* *INDENT-OFF* */
static const struct {
char *name;
- int str;
+ bool str;
} note_desc[] = {
[XEN_ELFNOTE_ENTRY] = { "ENTRY", 0},
[XEN_ELFNOTE_HYPERCALL_PAGE] = { "HYPERCALL_PAGE", 0},
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 6c43c34..798f88b 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -92,7 +92,7 @@ int elf_init(struct elf_binary *elf, const char *image_input,
size_t size)
}
#ifndef __XEN__
-void elf_call_log_callback(struct elf_binary *elf, int iserr,
+void elf_call_log_callback(struct elf_binary *elf, bool iserr,
const char *fmt,...) {
va_list al;
@@ -107,7 +107,7 @@ void elf_call_log_callback(struct elf_binary *elf, int
iserr,
}
void elf_set_log(struct elf_binary *elf, elf_log_callback *log_callback,
- void *log_caller_data, int verbose)
+ void *log_caller_data, bool verbose)
{
elf->log_callback = log_callback;
elf->log_caller_data = log_caller_data;
diff --git a/xen/common/libelf/libelf-private.h
b/xen/common/libelf/libelf-private.h
index 0bd9e66..ea7e197 100644
--- a/xen/common/libelf/libelf-private.h
+++ b/xen/common/libelf/libelf-private.h
@@ -77,7 +77,7 @@
#define elf_err(elf, fmt, args ... ) \
elf_call_log_callback(elf, 1, fmt , ## args );
-void elf_call_log_callback(struct elf_binary*, int iserr, const char *fmt,...);
+void elf_call_log_callback(struct elf_binary*, bool iserr, const char
*fmt,...);
#define safe_strcpy(d,s) \
do { strncpy((d),(s),sizeof((d))-1); \
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index b613593..0b7b2b6 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -31,7 +31,7 @@ const char *elf_check_broken(const struct elf_binary *elf)
return elf->broken;
}
-static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size,
+static bool elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size,
const void *region, uint64_t regionsize)
/*
* Returns true if the putative memory area [ptrval,ptrval+size>
@@ -53,7 +53,7 @@ static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t
size,
return 1;
}
-int elf_access_ok(struct elf_binary * elf,
+bool elf_access_ok(struct elf_binary * elf,
uint64_t ptrval, size_t size)
{
if ( elf_ptrval_in_range(ptrval, size, elf->image_base, elf->size) )
@@ -92,7 +92,7 @@ uint64_t elf_access_unsigned(struct elf_binary * elf,
elf_ptrval base,
uint64_t moreoffset, size_t size)
{
elf_ptrval ptrval = base + moreoffset;
- int need_swap = elf_swap(elf);
+ bool need_swap = elf_swap(elf);
const uint8_t *u8;
const uint16_t *u16;
const uint32_t *u32;
@@ -332,7 +332,7 @@ ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary
*elf, ELF_HANDLE_DECL(
/* ------------------------------------------------------------------------ */
-int elf_is_elfbinary(const void *image_start, size_t image_size)
+bool elf_is_elfbinary(const void *image_start, size_t image_size)
{
const Elf32_Ehdr *ehdr = image_start;
@@ -342,7 +342,7 @@ int elf_is_elfbinary(const void *image_start, size_t
image_size)
return IS_ELF(*ehdr);
}
-int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr)
+bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr)
{
uint64_t p_type = elf_uval(elf, phdr, p_type);
uint64_t p_flags = elf_uval(elf, phdr, p_flags);
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index c54c90b..1fa7421 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -29,6 +29,8 @@
#error define architectural endianness
#endif
+#include <stdbool.h>
+
#undef ELFSIZE
#include "elfstructs.h"
#ifdef __XEN__
@@ -42,7 +44,7 @@
struct elf_binary;
typedef void elf_log_callback(struct elf_binary*, void *caller_data,
- int iserr, const char *fmt, va_list al);
+ bool iserr, const char *fmt, va_list al);
#endif
@@ -236,7 +238,7 @@ struct elf_binary {
elf_log_callback *log_callback;
void *log_caller_data;
#endif
- int verbose;
+ bool verbose;
const char *broken;
};
@@ -300,8 +302,8 @@ void elf_memset_safe(struct elf_binary*, elf_ptrval dst, int
c, size_t);
* outside permitted areas.
*/
-int elf_access_ok(struct elf_binary * elf,
- uint64_t ptrval, size_t size);
+bool elf_access_ok(struct elf_binary * elf,
+ uint64_t ptrval, size_t size);
#define elf_store_val(elf, type, ptr, val) \
({ \
@@ -349,8 +351,8 @@ 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_start, size_t image_size);
-int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr);
+bool elf_is_elfbinary(const void *image_start, size_t image_size);
+bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr);
/* ------------------------------------------------------------------------ */
/* xc_libelf_loader.c */
@@ -364,7 +366,7 @@ int elf_init(struct elf_binary *elf, const char *image,
size_t size);
void elf_set_verbose(struct elf_binary *elf);
#else
void elf_set_log(struct elf_binary *elf, elf_log_callback*,
- void *log_caller_pointer, int verbose);
+ void *log_caller_pointer, bool verbose);
#endif
void elf_parse_binary(struct elf_binary *elf);
@@ -416,7 +418,7 @@ struct elf_dom_parms {
char xen_ver[16];
char loader[16];
int pae;
- int bsd_symtab;
+ bool bsd_symtab;
uint64_t virt_base;
uint64_t virt_entry;
uint64_t virt_hypercall;
--
1.7.2.5
Signed integers have undesirable undefined behaviours on overflow.
Malicious compilers can turn apparently-correct code into code with
security vulnerabilities etc.
So use only unsigned integers. Exceptions are booleans (which we have
already changed) and error codes.
We _do_ change all the chars which aren''t fixed constants from our own
text segment, but not the char*s. This is because it is safe to
access an arbitrary byte through a char*, but not necessarily safe to
convert an arbitrary value to a char.
As a consequence we need to compile libelf with -Wno-pointer-sign.
It is OK to change all the signed integers to unsigned because all the
inequalities in libelf are in contexts where we don''t
"expect"
negative numbers.
In libelf-dominfo.c:elf_xen_parse we rename a variable "rc" to
"more_notes" as it actually contains a note count derived from the
input image. The "error" return value from elf_xen_parse_notes is
changed from -1 to ~0U.
grepping shows only one occurrence of "PRId" or "%d" or
"%ld" in
libelf and xc_dom_elfloader.c (a "%d" which becomes "%u").
This is part of the fix to a security issue, XSA-55.
For those concerned about unintentional functional changes, the
following rune produces a version of the patch which is much smaller
and eliminates only non-functional changes:
GIT_EXTERNAL_DIFF=.../unsigned-differ git-diff <before>..<after>
where <before> and <after> are git refs for the code before and
after
this patch, and unsigned-differ is this shell script:
#!/bin/bash
set -e
seddery () {
perl -pe
''s/\b(?:elf_errorstatus|elf_negerrnoval)\b/int/g''
}
path="$1"
in="$2"
out="$5"
set +e
diff -pu --label "$path~" <(seddery <"$in")
--label "$path" <(seddery <"$out")
rc=$?
set -e
if [ $rc = 1 ]; then rc=0; fi
exit $rc
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: Introduce ELF_NOTE_INVALID, instead of using a literal ~0U.
v4: Fix regression in elf_round_up; use uint64_t here.
v3: Changes to booleans split off into separate patch.
v2: BUGFIX: Eliminate conversion to int of return from elf_xen_parse_notes.
BUGFIX: Fix the one printf format thing which needs changing.
Remove irrelevant change to constify note_desc.name in libelf-dominfo.c.
In xc_dom_load_elf_symtab change one sizeof(int) to sizeof(unsigned).
Do not change type of 2nd argument to memset.
Provide seddery for easier review.
Style fix.
---
tools/libxc/Makefile | 9 +++++-
tools/libxc/xc_dom.h | 7 +++--
tools/libxc/xc_dom_elfloader.c | 42 ++++++++++++++++-------------
tools/xcutils/readnotes.c | 15 +++++-----
xen/common/libelf/Makefile | 2 +
xen/common/libelf/libelf-dominfo.c | 52 ++++++++++++++++++-----------------
xen/common/libelf/libelf-loader.c | 20 +++++++-------
xen/common/libelf/libelf-tools.c | 24 ++++++++--------
xen/include/xen/libelf.h | 21 ++++++++------
9 files changed, 105 insertions(+), 87 deletions(-)
diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 4a31282..512a994 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -51,8 +51,13 @@ endif
vpath %.c ../../xen/common/libelf
CFLAGS += -I../../xen/common/libelf
-GUEST_SRCS-y += libelf-tools.c libelf-loader.c
-GUEST_SRCS-y += libelf-dominfo.c
+ELF_SRCS-y += libelf-tools.c libelf-loader.c
+ELF_SRCS-y += libelf-dominfo.c
+
+GUEST_SRCS-y += $(ELF_SRCS-y)
+
+$(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
+$(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
# new domain builder
GUEST_SRCS-y += xc_dom_core.c xc_dom_boot.c
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index c7eaf85..567913f 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -155,9 +155,10 @@ struct xc_dom_image {
struct xc_dom_loader {
char *name;
- int (*probe) (struct xc_dom_image * dom);
- int (*parser) (struct xc_dom_image * dom);
- int (*loader) (struct xc_dom_image * dom);
+ /* Sadly the error returns from these functions are not consistent: */
+ elf_negerrnoval (*probe) (struct xc_dom_image * dom);
+ elf_negerrnoval (*parser) (struct xc_dom_image * dom);
+ elf_errorstatus (*loader) (struct xc_dom_image * dom);
struct xc_dom_loader *next;
};
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 8f9c2fb..eb2e3d2 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -82,7 +82,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
/* ------------------------------------------------------------------------ */
/* parse elf binary */
-static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
+static elf_negerrnoval check_elf_kernel(struct xc_dom_image *dom, bool verbose)
{
if ( dom->kernel_blob == NULL )
{
@@ -104,12 +104,12 @@ static int check_elf_kernel(struct xc_dom_image *dom, bool
verbose)
return 0;
}
-static int xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
+static elf_negerrnoval xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
{
return check_elf_kernel(dom, 0);
}
-static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
+static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom,
struct elf_binary *elf, bool load)
{
struct elf_binary syms;
@@ -117,7 +117,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
xen_vaddr_t symtab, maxaddr;
ELF_PTRVAL_CHAR hdr;
size_t size;
- int h, count, type, i, tables = 0;
+ unsigned h, count, type, i, tables = 0;
if ( elf_swap(elf) )
{
@@ -138,13 +138,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
elf->caller_xdest_base = hdr_ptr;
elf->caller_xdest_size = allow_size;
hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
- elf_store_val(elf, int, hdr, size - sizeof(int));
+ elf_store_val(elf, unsigned, hdr, size - sizeof(unsigned));
}
else
{
char *hdr_ptr;
- size = sizeof(int) + elf_size(elf, elf->ehdr) +
+ size = sizeof(unsigned) + elf_size(elf, elf->ehdr) +
elf_shdr_count(elf) * elf_size(elf, shdr);
hdr_ptr = xc_dom_malloc(dom, size);
if ( hdr_ptr == NULL )
@@ -155,15 +155,15 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
}
- elf_memcpy_safe(elf, hdr + sizeof(int),
+ elf_memcpy_safe(elf, hdr + sizeof(unsigned),
ELF_IMAGE_BASE(elf),
elf_size(elf, elf->ehdr));
- elf_memcpy_safe(elf, hdr + sizeof(int) + elf_size(elf, elf->ehdr),
+ elf_memcpy_safe(elf, hdr + sizeof(unsigned) + elf_size(elf, elf->ehdr),
ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
elf_shdr_count(elf) * elf_size(elf, shdr));
if ( elf_64bit(elf) )
{
- Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(int));
+ Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(unsigned));
ehdr->e_phoff = 0;
ehdr->e_phentsize = 0;
ehdr->e_phnum = 0;
@@ -172,22 +172,22 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
}
else
{
- Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(int));
+ Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(unsigned));
ehdr->e_phoff = 0;
ehdr->e_phentsize = 0;
ehdr->e_phnum = 0;
ehdr->e_shoff = elf_size(elf, elf->ehdr);
ehdr->e_shstrndx = SHN_UNDEF;
}
- if ( elf->caller_xdest_size < sizeof(int) )
+ if ( elf->caller_xdest_size < sizeof(unsigned) )
{
DOMPRINTF("%s/%s: header size %"PRIx64" too small",
__FUNCTION__, load ? "load" : "parse",
(uint64_t)elf->caller_xdest_size);
return -1;
}
- if ( elf_init(&syms, elf->caller_xdest_base + sizeof(int),
- elf->caller_xdest_size - sizeof(int)) )
+ if ( elf_init(&syms, elf->caller_xdest_base + sizeof(unsigned),
+ elf->caller_xdest_size - sizeof(unsigned)) )
return -1;
/*
@@ -207,7 +207,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
xc_elf_set_logfile(dom->xch, &syms, 1);
- symtab = dom->bsd_symtab_start + sizeof(int);
+ symtab = dom->bsd_symtab_start + sizeof(unsigned);
maxaddr = elf_round_up(&syms, symtab + elf_size(&syms, syms.ehdr) +
elf_shdr_count(&syms) * elf_size(&syms,
shdr));
@@ -253,7 +253,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
size = elf_uval(&syms, shdr, sh_size);
maxaddr = elf_round_up(&syms, maxaddr + size);
tables++;
- DOMPRINTF("%s: h=%d %s, size=0x%zx, maxaddr=0x%" PRIx64
"",
+ DOMPRINTF("%s: h=%u %s, size=0x%zx, maxaddr=0x%" PRIx64
"",
__FUNCTION__, h,
type == SHT_SYMTAB ? "symtab" :
"strtab",
size, maxaddr);
@@ -292,10 +292,14 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
return 0;
}
-static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
+static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
+ /*
+ * This function sometimes returns -1 for error and sometimes
+ * an errno value. WTF?
+ */
{
struct elf_binary *elf;
- int rc;
+ elf_errorstatus rc;
rc = check_elf_kernel(dom, 1);
if ( rc != 0 )
@@ -356,10 +360,10 @@ out:
return rc;
}
-static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
+static elf_errorstatus xc_dom_load_elf_kernel(struct xc_dom_image *dom)
{
struct elf_binary *elf = dom->private_loader;
- int rc;
+ elf_errorstatus rc;
xen_pfn_t pages;
elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg,
&pages);
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 4394905..88a83fa 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -70,7 +70,7 @@ static void print_numeric_note(const char *prefix, struct
elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
uint64_t value = elf_note_numeric(elf, note);
- int descsz = elf_uval(elf, note, descsz);
+ unsigned descsz = elf_uval(elf, note, descsz);
printf("%s: %#*" PRIx64 " (%d bytes)\n",
prefix, 2+2*descsz, value, descsz);
@@ -79,7 +79,7 @@ static void print_numeric_note(const char *prefix, struct
elf_binary *elf,
static void print_l1_mfn_valid_note(const char *prefix, struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
- int descsz = elf_uval(elf, note, descsz);
+ unsigned descsz = elf_uval(elf, note, descsz);
ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
/* XXX should be able to cope with a list of values. */
@@ -99,10 +99,10 @@ static void print_l1_mfn_valid_note(const char *prefix,
struct elf_binary *elf,
}
-static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start,
ELF_HANDLE_DECL(elf_note) end)
+static unsigned print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
start, ELF_HANDLE_DECL(elf_note) end)
{
ELF_HANDLE_DECL(elf_note) note;
- int notes_found = 0;
+ unsigned notes_found = 0;
const char *this_note_name;
for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end); note
= elf_note_next(elf, note) )
@@ -160,7 +160,7 @@ static int print_notes(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) start,
break;
default:
printf("unknown note type %#x\n",
- (int)elf_uval(elf, note, type));
+ (unsigned)elf_uval(elf, note, type));
break;
}
}
@@ -170,12 +170,13 @@ static int print_notes(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) start,
int main(int argc, char **argv)
{
const char *f;
- int fd,h,size,usize,count;
+ int fd;
+ unsigned h,size,usize,count;
void *image,*tmp;
struct stat st;
struct elf_binary elf;
ELF_HANDLE_DECL(elf_shdr) shdr;
- int notes_found = 0;
+ unsigned notes_found = 0;
struct setup_header *hdr;
uint64_t payload_offset, payload_length;
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 18dc8e2..5bf8f76 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -2,6 +2,8 @@ obj-bin-y := libelf.o
SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
+CFLAGS += -Wno-pointer-sign
+
libelf.o: libelf-temp.o Makefile
$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index c4ced67..0b07002 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -29,15 +29,15 @@ static const char *const elf_xen_feature_names[] = {
[XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
[XENFEAT_dom0] = "dom0"
};
-static const int elf_xen_features +static const unsigned elf_xen_features
sizeof(elf_xen_feature_names) / sizeof(elf_xen_feature_names[0]);
-int elf_xen_parse_features(const char *features,
+elf_errorstatus elf_xen_parse_features(const char *features,
uint32_t *supported,
uint32_t *required)
{
- char feature[64];
- int pos, len, i;
+ unsigned char feature[64];
+ unsigned pos, len, i;
if ( features == NULL )
return 0;
@@ -94,7 +94,7 @@ int elf_xen_parse_features(const char *features,
/* ------------------------------------------------------------------------ */
/* xen elf notes */
-int elf_xen_parse_note(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
struct elf_dom_parms *parms,
ELF_HANDLE_DECL(elf_note) note)
{
@@ -125,7 +125,7 @@ int elf_xen_parse_note(struct elf_binary *elf,
const char *str = NULL;
uint64_t val = 0;
unsigned int i;
- int type = elf_uval(elf, note, type);
+ unsigned type = elf_uval(elf, note, type);
if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
(note_desc[type].name == NULL) )
@@ -216,12 +216,14 @@ int elf_xen_parse_note(struct elf_binary *elf,
return 0;
}
-static int elf_xen_parse_notes(struct elf_binary *elf,
+#define ELF_NOTE_INVALID (~0U)
+
+static unsigned elf_xen_parse_notes(struct elf_binary *elf,
struct elf_dom_parms *parms,
ELF_PTRVAL_CONST_VOID start,
ELF_PTRVAL_CONST_VOID end)
{
- int xen_elfnotes = 0;
+ unsigned xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
const char *note_name;
@@ -237,7 +239,7 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
if ( strcmp(note_name, "Xen") )
continue;
if ( elf_xen_parse_note(elf, parms, note) )
- return -1;
+ return ELF_NOTE_INVALID;
xen_elfnotes++;
}
return xen_elfnotes;
@@ -246,12 +248,12 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
/* ------------------------------------------------------------------------ */
/* __xen_guest section */
-int elf_xen_parse_guest_info(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
ELF_PTRVAL_CONST_CHAR h;
- char name[32], value[128];
- int len;
+ unsigned char name[32], value[128];
+ unsigned len;
h = parms->guest_info;
#define STAR(h) (elf_access_unsigned(elf, (h), 0, 1))
@@ -334,13 +336,13 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
/* ------------------------------------------------------------------------ */
/* sanity checks */
-static int elf_xen_note_check(struct elf_binary *elf,
+static elf_errorstatus elf_xen_note_check(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) &&
(ELF_PTRVAL_INVALID(parms->guest_info)) )
{
- int machine = elf_uval(elf, elf->ehdr, e_machine);
+ unsigned machine = elf_uval(elf, elf->ehdr, e_machine);
if ( (machine == EM_386) || (machine == EM_X86_64) )
{
elf_err(elf, "%s: ERROR: Not a Xen-ELF image: "
@@ -378,7 +380,7 @@ static int elf_xen_note_check(struct elf_binary *elf,
return 0;
}
-static int elf_xen_addr_calc_check(struct elf_binary *elf,
+static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
if ( (parms->elf_paddr_offset != UNSET_ADDR) &&
@@ -464,13 +466,13 @@ static int elf_xen_addr_calc_check(struct elf_binary *elf,
/* ------------------------------------------------------------------------ */
/* glue it all together ... */
-int elf_xen_parse(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
ELF_HANDLE_DECL(elf_shdr) shdr;
ELF_HANDLE_DECL(elf_phdr) phdr;
- int xen_elfnotes = 0;
- int i, count, rc;
+ unsigned xen_elfnotes = 0;
+ unsigned i, count, more_notes;
elf_memset_unchecked(parms, 0, sizeof(*parms));
parms->virt_base = UNSET_ADDR;
@@ -495,13 +497,13 @@ int elf_xen_parse(struct elf_binary *elf,
if (elf_uval(elf, phdr, p_offset) == 0)
continue;
- rc = elf_xen_parse_notes(elf, parms,
+ more_notes = elf_xen_parse_notes(elf, parms,
elf_segment_start(elf, phdr),
elf_segment_end(elf, phdr));
- if ( rc == -1 )
+ if ( more_notes == ELF_NOTE_INVALID )
return -1;
- xen_elfnotes += rc;
+ xen_elfnotes += more_notes;
}
/*
@@ -518,17 +520,17 @@ int elf_xen_parse(struct elf_binary *elf,
if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
continue;
- rc = elf_xen_parse_notes(elf, parms,
+ more_notes = elf_xen_parse_notes(elf, parms,
elf_section_start(elf, shdr),
elf_section_end(elf, shdr));
- if ( rc == -1 )
+ if ( more_notes == ELF_NOTE_INVALID )
return -1;
- if ( xen_elfnotes == 0 && rc > 0 )
+ if ( xen_elfnotes == 0 && more_notes > 0 )
elf_msg(elf, "%s: using notes from SHT_NOTE
section\n", __FUNCTION__);
- xen_elfnotes += rc;
+ xen_elfnotes += more_notes;
}
}
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 798f88b..937c99b 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -24,7 +24,7 @@
/* ------------------------------------------------------------------------ */
-int elf_init(struct elf_binary *elf, const char *image_input, size_t size)
+elf_errorstatus 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;
@@ -114,7 +114,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback
*log_callback,
elf->verbose = verbose;
}
-static int elf_load_image(struct elf_binary *elf,
+static elf_errorstatus elf_load_image(struct elf_binary *elf,
ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src,
uint64_t filesz, uint64_t memsz)
{
@@ -129,9 +129,9 @@ void elf_set_verbose(struct elf_binary *elf)
elf->verbose = 1;
}
-static int elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst,
ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
+static elf_errorstatus elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID
dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
{
- int rc;
+ elf_errorstatus rc;
if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
return -1;
/* We trust the dom0 kernel image completely, so we don''t care
@@ -151,7 +151,7 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t
pstart)
{
uint64_t sz;
ELF_HANDLE_DECL(elf_shdr) shdr;
- int i, type;
+ unsigned i, type;
if ( !ELF_HANDLE_VALID(elf->sym_tab) )
return;
@@ -187,7 +187,7 @@ static void elf_load_bsdsyms(struct elf_binary *elf)
ELF_PTRVAL_VOID symbase;
ELF_PTRVAL_VOID symtab_addr;
ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr;
- int i, type;
+ unsigned i, type;
if ( !elf->bsd_symtab_pstart )
return;
@@ -220,7 +220,7 @@ do { \
elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(shdr),
ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
sz);
- maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
+ maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva +
sz);
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
@@ -233,10 +233,10 @@ do { \
elf_memcpy_safe(elf, maxva, elf_section_start(elf, shdr), sz);
/* Mangled to be based on ELF header location. */
elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
- maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva +
sz);
+ maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned
long)maxva + sz);
}
shdr = ELF_MAKE_HANDLE(elf_shdr, ELF_HANDLE_PTRVAL(shdr) +
- (long)elf_uval(elf, elf->ehdr, e_shentsize));
+ (unsigned long)elf_uval(elf, elf->ehdr,
e_shentsize));
}
/* Write down the actual sym size. */
@@ -273,7 +273,7 @@ void elf_parse_binary(struct elf_binary *elf)
__FUNCTION__, elf->pstart, elf->pend);
}
-int elf_load_binary(struct elf_binary *elf)
+elf_errorstatus elf_load_binary(struct elf_binary *elf)
{
ELF_HANDLE_DECL(elf_phdr) phdr;
uint64_t i, count, paddr, offset, filesz, memsz;
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 0b7b2b6..6543f33 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -122,19 +122,19 @@ uint64_t elf_access_unsigned(struct elf_binary * elf,
elf_ptrval base,
uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr)
{
- int elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
+ uint64_t elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
return (addr + elf_round) & ~elf_round;
}
/* ------------------------------------------------------------------------ */
-int elf_shdr_count(struct elf_binary *elf)
+unsigned elf_shdr_count(struct elf_binary *elf)
{
return elf_uval(elf, elf->ehdr, e_shnum);
}
-int elf_phdr_count(struct elf_binary *elf)
+unsigned elf_phdr_count(struct elf_binary *elf)
{
return elf_uval(elf, elf->ehdr, e_phnum);
}
@@ -144,7 +144,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
uint64_t count = elf_shdr_count(elf);
ELF_HANDLE_DECL(elf_shdr) shdr;
const char *sname;
- int i;
+ unsigned i;
for ( i = 0; i < count; i++ )
{
@@ -156,7 +156,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
return ELF_INVALID_HANDLE(elf_shdr);
}
-ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned
index)
{
uint64_t count = elf_shdr_count(elf);
ELF_PTRVAL_CONST_VOID ptr;
@@ -170,7 +170,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct
elf_binary *elf, int index)
return ELF_MAKE_HANDLE(elf_shdr, ptr);
}
-ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned
index)
{
uint64_t count = elf_uval(elf, elf->ehdr, e_phnum);
ELF_PTRVAL_CONST_VOID ptr;
@@ -264,7 +264,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
return ELF_INVALID_HANDLE(elf_sym);
}
-ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned
index)
{
ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
ELF_HANDLE_DECL(elf_sym) sym;
@@ -280,7 +280,7 @@ const char *elf_note_name(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
- int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
+ unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz;
}
@@ -288,7 +288,7 @@ ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_
uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
- int descsz = elf_uval(elf, note, descsz);
+ unsigned descsz = elf_uval(elf, note, descsz);
switch (descsz)
{
@@ -306,7 +306,7 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note
unsigned int unitsz, unsigned int idx)
{
ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
- int descsz = elf_uval(elf, note, descsz);
+ unsigned descsz = elf_uval(elf, note, descsz);
if ( descsz % unitsz || idx >= descsz / unitsz )
return 0;
@@ -324,8 +324,8 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note
ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
- int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
- int descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
+ unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
+ unsigned descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
return ELF_MAKE_HANDLE(elf_note, ELF_HANDLE_PTRVAL(note) + elf_size(elf,
note) + namesz + descsz);
}
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 1fa7421..e4588de 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -31,6 +31,9 @@
#include <stdbool.h>
+typedef int elf_errorstatus; /* 0: ok; -ve (normally -1): error */
+typedef int elf_negerrnoval; /* 0: ok; -EFOO: error */
+
#undef ELFSIZE
#include "elfstructs.h"
#ifdef __XEN__
@@ -327,12 +330,12 @@ bool elf_access_ok(struct elf_binary * elf,
/* ------------------------------------------------------------------------ */
/* xc_libelf_tools.c */
-int elf_shdr_count(struct elf_binary *elf);
-int elf_phdr_count(struct elf_binary *elf);
+unsigned elf_shdr_count(struct elf_binary *elf);
+unsigned elf_phdr_count(struct elf_binary *elf);
ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char
*name);
-ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index);
-ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned
index);
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned
index);
const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr); /* might return NULL if inputs are invalid */
ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
@@ -342,7 +345,7 @@ ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary
*elf, ELF_HANDLE_DECL(
ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr);
ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol);
-ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned
index);
const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note); /* may return NULL */
ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note);
@@ -357,7 +360,7 @@ bool elf_phdr_is_loadable(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr
/* ------------------------------------------------------------------------ */
/* xc_libelf_loader.c */
-int elf_init(struct elf_binary *elf, const char *image, size_t size);
+elf_errorstatus elf_init(struct elf_binary *elf, const char *image, size_t
size);
/*
* image and size must be correct. They will be recorded in
* *elf, and must remain valid while the elf is in use.
@@ -370,7 +373,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback*,
#endif
void elf_parse_binary(struct elf_binary *elf);
-int elf_load_binary(struct elf_binary *elf);
+elf_errorstatus elf_load_binary(struct elf_binary *elf);
ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr);
uint64_t elf_lookup_addr(struct elf_binary *elf, const char *symbol);
@@ -383,7 +386,7 @@ const char *elf_check_broken(const struct elf_binary *elf);
/* NULL means OK */
/* ------------------------------------------------------------------------ */
/* xc_libelf_relocate.c */
-int elf_reloc(struct elf_binary *elf);
+elf_errorstatus elf_reloc(struct elf_binary *elf);
/* ------------------------------------------------------------------------ */
/* xc_libelf_dominfo.c */
@@ -417,7 +420,7 @@ struct elf_dom_parms {
char guest_ver[16];
char xen_ver[16];
char loader[16];
- int pae;
+ int pae; /* some kind of enum apparently */
bool bsd_symtab;
uint64_t virt_base;
uint64_t virt_entry;
--
1.7.2.5
Ensure that libelf does not have any loops which can run away
indefinitely even if the input is bogus. (Grepped for \bfor, \bwhile
and \bgoto in libelf and xc_dom_*loader*.c.)
Changes needed:
* elf_note_next uses the note''s unchecked alleged length, which might
wrap round. If it does, return ELF_MAX_PTRVAL (0xfff..fff) instead,
which will be beyond the end of the section and so terminate the
caller''s loop. Also check that the returned psuedopointer is sane.
* In various loops over section and program headers, check that the
calculated header pointer is still within the image, and quit the
loop if it isn''t.
* Some fixed limits to avoid potentially O(image_size^2) loops:
- maximum length of strings: 4K (longer ones ignored totally)
- maximum total number of ELF notes: 65536 (any more are ignored)
* Check that the total program contents (text, data) we copy or
initialise doesn''t exceed twice the output image area size.
* Remove an entirely useless loop from elf_xen_parse (!)
* Replace a nested search loop in in xc_dom_load_elf_symtab in
xc_dom_elfloader.c by a precomputation of a bitmap of referenced
symtabs.
We have not changed loops which might, in principle, iterate over the
whole image - even if they might do so one byte at a time with a
nontrivial access check function in the middle.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: Fix regression due to wrong image size loop limit calculation.
Check return value from xc_dom_malloc.
v4: Fix regression due to misplacement of test in elf_shdr_by_name
(uninitialised variable).
Introduce fixed limits.
Avoid O(size^2) loops.
Check returned psuedopointer from elf_note_next is correct.
A few style fixes.
v3: Fix a whitespace error.
v2: BUGFIX: elf_shdr_by_name, elf_note_next: Reject new <= old, not just
<.
elf_shdr_by_name: Change order of checks to be a bit clearer.
elf_load_bsdsyms: shdr loop check, improve chance of brokenness detection.
Style fixes.
---
tools/libxc/xc_dom_elfloader.c | 33 ++++++++++++++++++------
xen/common/libelf/libelf-dominfo.c | 48 ++++++++++++++++++++++++------------
xen/common/libelf/libelf-loader.c | 47 +++++++++++++++++++++++++++++++++-
xen/common/libelf/libelf-tools.c | 28 +++++++++++++++++++-
xen/include/xen/libelf.h | 3 ++
5 files changed, 130 insertions(+), 29 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index eb2e3d2..81c2519 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -28,6 +28,7 @@
#include "xg_private.h"
#include "xc_dom.h"
+#include "xc_bitops.h"
#define XEN_VER "xen-3.0"
@@ -118,6 +119,7 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
ELF_PTRVAL_CHAR hdr;
size_t size;
unsigned h, count, type, i, tables = 0;
+ unsigned long *strtab_referenced = NULL;
if ( elf_swap(elf) )
{
@@ -218,22 +220,35 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
symtab, maxaddr);
count = elf_shdr_count(&syms);
+ /* elf_shdr_count guarantees that count is reasonable */
+
+ strtab_referenced = xc_dom_malloc(dom, bitmap_size(count));
+ if ( strtab_referenced == NULL )
+ return -1;
+ bitmap_clear(strtab_referenced, count);
+ /* Note the symtabs @h linked to by any strtab @i. */
+ for ( i = 0; i < count; i++ )
+ {
+ shdr2 = elf_shdr_by_index(&syms, i);
+ if ( elf_uval(&syms, shdr2, sh_type) == SHT_SYMTAB )
+ {
+ h = elf_uval(&syms, shdr2, sh_link);
+ if (h < count)
+ set_bit(h, strtab_referenced);
+ }
+ }
+
for ( h = 0; h < count; h++ )
{
shdr = ELF_OBSOLETE_VOIDP_CAST elf_shdr_by_index(&syms, h);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+ /* input has an insane section header count field */
+ break;
type = elf_uval(&syms, shdr, sh_type);
if ( type == SHT_STRTAB )
{
- /* Look for a strtab @i linked to symtab @h. */
- for ( i = 0; i < count; i++ )
- {
- shdr2 = elf_shdr_by_index(&syms, i);
- if ( (elf_uval(&syms, shdr2, sh_type) == SHT_SYMTAB)
&&
- (elf_uval(&syms, shdr2, sh_link) == h) )
- break;
- }
/* Skip symtab @h if we found no corresponding strtab @i. */
- if ( i == count )
+ if ( !test_bit(h, strtab_referenced) )
{
if ( elf_64bit(&syms) )
elf_store_field(elf, shdr, e64.sh_offset, 0);
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index 0b07002..b0ba4d8 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -221,7 +221,8 @@ elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
static unsigned elf_xen_parse_notes(struct elf_binary *elf,
struct elf_dom_parms *parms,
ELF_PTRVAL_CONST_VOID start,
- ELF_PTRVAL_CONST_VOID end)
+ ELF_PTRVAL_CONST_VOID end,
+ unsigned *total_note_count)
{
unsigned xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
@@ -233,6 +234,12 @@ static unsigned elf_xen_parse_notes(struct elf_binary *elf,
ELF_HANDLE_PTRVAL(note) < parms->elf_note_end;
note = elf_note_next(elf, note) )
{
+ if ( *total_note_count >= ELF_MAX_TOTAL_NOTE_COUNT )
+ {
+ elf_mark_broken(elf, "too many ELF notes");
+ break;
+ }
+ (*total_note_count)++;
note_name = elf_note_name(elf, note);
if ( note_name == NULL )
continue;
@@ -473,6 +480,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr;
unsigned xen_elfnotes = 0;
unsigned i, count, more_notes;
+ unsigned total_note_count = 0;
elf_memset_unchecked(parms, 0, sizeof(*parms));
parms->virt_base = UNSET_ADDR;
@@ -487,6 +495,13 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
for ( i = 0; i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
+ /*
+ * This test also arranges for the loop to terminate if the
+ * input file has a ridiculous value for the header count: The
+ * first putative header outside the input image will appear
+ * to have type 0 (since out-of-range accesses read as 0) and
+ * PT_NOTE != 0.
+ */
if ( elf_uval(elf, phdr, p_type) != PT_NOTE )
continue;
@@ -499,7 +514,8 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
more_notes = elf_xen_parse_notes(elf, parms,
elf_segment_start(elf, phdr),
- elf_segment_end(elf, phdr));
+ elf_segment_end(elf, phdr),
+ &total_note_count);
if ( more_notes == ELF_NOTE_INVALID )
return -1;
@@ -517,12 +533,17 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
{
shdr = elf_shdr_by_index(elf, i);
+ /*
+ * See above re guarantee of loop termination.
+ * SHT_NOTE != 0.
+ */
if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
continue;
more_notes = elf_xen_parse_notes(elf, parms,
elf_section_start(elf, shdr),
- elf_section_end(elf, shdr));
+ elf_section_end(elf, shdr),
+ &total_note_count);
if ( more_notes == ELF_NOTE_INVALID )
return -1;
@@ -540,20 +561,15 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
*/
if ( xen_elfnotes == 0 )
{
- count = elf_shdr_count(elf);
- for ( i = 0; i < count; i++ )
+ shdr = elf_shdr_by_name(elf, "__xen_guest");
+ if ( ELF_HANDLE_VALID(shdr) )
{
- shdr = elf_shdr_by_name(elf, "__xen_guest");
- if ( ELF_HANDLE_VALID(shdr) )
- {
- parms->guest_info = elf_section_start(elf, shdr);
- parms->elf_note_start = ELF_INVALID_PTRVAL;
- parms->elf_note_end = ELF_INVALID_PTRVAL;
- elf_msg(elf, "%s: __xen_guest: \"%s\"\n",
__FUNCTION__,
- elf_strfmt(elf, parms->guest_info));
- elf_xen_parse_guest_info(elf, parms);
- break;
- }
+ parms->guest_info = elf_section_start(elf, shdr);
+ parms->elf_note_start = ELF_INVALID_PTRVAL;
+ parms->elf_note_end = ELF_INVALID_PTRVAL;
+ elf_msg(elf, "%s: __xen_guest: \"%s\"\n",
__FUNCTION__,
+ elf_strfmt(elf, parms->guest_info));
+ elf_xen_parse_guest_info(elf, parms);
}
}
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 937c99b..47957aa 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -75,6 +75,9 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char
*image_input, size_t
for ( i = 0; i < count; i++ )
{
shdr = elf_shdr_by_index(elf, i);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+ /* input has an insane section header count field */
+ break;
if ( elf_uval(elf, shdr, sh_type) != SHT_SYMTAB )
continue;
elf->sym_tab = shdr;
@@ -170,6 +173,9 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t
pstart)
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
shdr = elf_shdr_by_index(elf, i);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+ /* input has an insane section header count field */
+ break;
type = elf_uval(elf, shdr, sh_type);
if ( (type == SHT_STRTAB) || (type == SHT_SYMTAB) )
sz = elf_round_up(elf, sz + elf_uval(elf, shdr, sh_size));
@@ -224,6 +230,9 @@ do { \
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
+ elf_ptrval old_shdr_p;
+ elf_ptrval new_shdr_p;
+
type = elf_uval(elf, shdr, sh_type);
if ( (type == SHT_STRTAB) || (type == SHT_SYMTAB) )
{
@@ -235,8 +244,16 @@ do { \
elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned
long)maxva + sz);
}
- shdr = ELF_MAKE_HANDLE(elf_shdr, ELF_HANDLE_PTRVAL(shdr) +
- (unsigned long)elf_uval(elf, elf->ehdr,
e_shentsize));
+ old_shdr_p = ELF_HANDLE_PTRVAL(shdr);
+ new_shdr_p = old_shdr_p + elf_uval(elf, elf->ehdr, e_shentsize);
+ if ( new_shdr_p <= old_shdr_p ) /* wrapped or stuck */
+ {
+ elf_mark_broken(elf, "bad section header length");
+ break;
+ }
+ if ( !elf_access_ok(elf, new_shdr_p, 1) ) /* outside image */
+ break;
+ shdr = ELF_MAKE_HANDLE(elf_shdr, new_shdr_p);
}
/* Write down the actual sym size. */
@@ -256,6 +273,9 @@ void elf_parse_binary(struct elf_binary *elf)
for ( i = 0; i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
+ /* input has an insane program header count field */
+ break;
if ( !elf_phdr_is_loadable(elf, phdr) )
continue;
paddr = elf_uval(elf, phdr, p_paddr);
@@ -278,11 +298,20 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
ELF_HANDLE_DECL(elf_phdr) phdr;
uint64_t i, count, paddr, offset, filesz, memsz;
ELF_PTRVAL_VOID dest;
+ /*
+ * Let bizarre ELFs write the output image up to twice; this
+ * calculation is just to ensure our copying loop is no worse than
+ * O(domain_size).
+ */
+ uint64_t remain_allow_copy = (uint64_t)elf->dest_size * 2;
count = elf_uval(elf, elf->ehdr, e_phnum);
for ( i = 0; i < count; i++ )
{
phdr = elf_phdr_by_index(elf, i);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(phdr), 1) )
+ /* input has an insane program header count field */
+ break;
if ( !elf_phdr_is_loadable(elf, phdr) )
continue;
paddr = elf_uval(elf, phdr, p_paddr);
@@ -290,6 +319,20 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
filesz = elf_uval(elf, phdr, p_filesz);
memsz = elf_uval(elf, phdr, p_memsz);
dest = elf_get_ptr(elf, paddr);
+
+ /*
+ * We need to check that the input image doesn''t have us copy
+ * the whole image zillions of times, as that could lead to
+ * O(n^2) time behaviour and possible DoS by a malicous ELF.
+ */
+ if ( remain_allow_copy < memsz )
+ {
+ elf_mark_broken(elf, "program segments total to more"
+ " than the input image size");
+ break;
+ }
+ remain_allow_copy -= memsz;
+
elf_msg(elf, "%s: phdr %" PRIu64 " at
0x%"ELF_PRPTRVAL" -> 0x%"ELF_PRPTRVAL"\n",
__func__, i, dest, (ELF_PTRVAL_VOID)(dest + filesz));
if ( elf_load_image(elf, dest, ELF_IMAGE_BASE(elf) + offset, filesz,
memsz) != 0 )
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 6543f33..ef13b0d 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -131,7 +131,16 @@ uint64_t elf_round_up(struct elf_binary *elf, uint64_t
addr)
unsigned elf_shdr_count(struct elf_binary *elf)
{
- return elf_uval(elf, elf->ehdr, e_shnum);
+ unsigned count = elf_uval(elf, elf->ehdr, e_shnum);
+ uint64_t max = elf->size / sizeof(Elf32_Shdr);
+ if (max > ~(unsigned)0)
+ max = ~(unsigned)0; /* Xen doesn''t have limits.h :-/ */
+ if (count > max)
+ {
+ elf_mark_broken(elf, "far too many section headers");
+ count = max;
+ }
+ return count;
}
unsigned elf_phdr_count(struct elf_binary *elf)
@@ -149,6 +158,9 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
for ( i = 0; i < count; i++ )
{
shdr = elf_shdr_by_index(elf, i);
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+ /* input has an insane section header count field */
+ break;
sname = elf_section_name(elf, shdr);
if ( sname && !strcmp(sname, name) )
return shdr;
@@ -204,6 +216,11 @@ const char *elf_strval(struct elf_binary *elf, elf_ptrval
start)
if ( !elf_access_unsigned(elf, start, length, 1) )
/* ok */
return ELF_UNSAFE_PTR(start);
+ if ( length >= ELF_MAX_STRING_LENGTH )
+ {
+ elf_mark_broken(elf, "excessively long string");
+ return NULL;
+ }
}
}
@@ -327,7 +344,14 @@ ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary
*elf, ELF_HANDLE_DECL(
unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
unsigned descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
- return ELF_MAKE_HANDLE(elf_note, ELF_HANDLE_PTRVAL(note) + elf_size(elf,
note) + namesz + descsz);
+ elf_ptrval ptrval = ELF_HANDLE_PTRVAL(note)
+ + elf_size(elf, note) + namesz + descsz;
+
+ if ( ( ptrval <= ELF_HANDLE_PTRVAL(note) || /* wrapped or stuck */
+ !elf_access_ok(elf, ELF_HANDLE_PTRVAL(note), 1) ) )
+ ptrval = ELF_MAX_PTRVAL; /* terminate caller''s loop */
+
+ return ELF_MAKE_HANDLE(elf_note, ptrval);
}
/* ------------------------------------------------------------------------ */
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index e4588de..4ca6b8f 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -51,6 +51,9 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
#endif
+#define ELF_MAX_STRING_LENGTH 4096
+#define ELF_MAX_TOTAL_NOTE_COUNT 65536
+
/* ------------------------------------------------------------------------ */
/* Macros for accessing the input image and output area. */
--
1.7.2.5
Abolish ELF_PTRVAL_[CONST_]{CHAR,VOID}; change uses to elf_ptrval.
Abolish ELF_HANDLE_DECL_NONCONST; change uses to ELF_HANDLE_DECL.
Abolish ELF_OBSOLETE_VOIDP_CAST; simply remove all uses.
No functional change. (Verified by diffing assembler output.)
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>
v2: New patch.
---
tools/libxc/xc_dom_elfloader.c | 8 +++---
tools/xcutils/readnotes.c | 2 +-
xen/common/libelf/libelf-dominfo.c | 6 ++--
xen/common/libelf/libelf-loader.c | 24 +++++++++---------
xen/common/libelf/libelf-tools.c | 24 +++++++++---------
xen/include/xen/libelf.h | 48 +++++++++---------------------------
6 files changed, 44 insertions(+), 68 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 81c2519..1d2727e 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -114,9 +114,9 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
struct elf_binary *elf, bool load)
{
struct elf_binary syms;
- ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr; ELF_HANDLE_DECL(elf_shdr) shdr2;
+ ELF_HANDLE_DECL(elf_shdr) shdr; ELF_HANDLE_DECL(elf_shdr) shdr2;
xen_vaddr_t symtab, maxaddr;
- ELF_PTRVAL_CHAR hdr;
+ elf_ptrval hdr;
size_t size;
unsigned h, count, type, i, tables = 0;
unsigned long *strtab_referenced = NULL;
@@ -240,7 +240,7 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
for ( h = 0; h < count; h++ )
{
- shdr = ELF_OBSOLETE_VOIDP_CAST elf_shdr_by_index(&syms, h);
+ shdr = elf_shdr_by_index(&syms, h);
if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
/* input has an insane section header count field */
break;
@@ -276,7 +276,7 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
if ( load )
{
shdr2 = elf_shdr_by_index(elf, h);
- elf_memcpy_safe(elf, ELF_OBSOLETE_VOIDP_CAST
elf_section_start(&syms, shdr),
+ elf_memcpy_safe(elf, elf_section_start(&syms, shdr),
elf_section_start(elf, shdr2),
size);
}
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 88a83fa..76c3226 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -80,7 +80,7 @@ static void print_l1_mfn_valid_note(const char *prefix, struct
elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
{
unsigned descsz = elf_uval(elf, note, descsz);
- ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
+ elf_ptrval desc = elf_note_desc(elf, note);
/* XXX should be able to cope with a list of values. */
switch ( descsz / 2 )
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index b0ba4d8..c368f24 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -220,8 +220,8 @@ elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
static unsigned elf_xen_parse_notes(struct elf_binary *elf,
struct elf_dom_parms *parms,
- ELF_PTRVAL_CONST_VOID start,
- ELF_PTRVAL_CONST_VOID end,
+ elf_ptrval start,
+ elf_ptrval end,
unsigned *total_note_count)
{
unsigned xen_elfnotes = 0;
@@ -258,7 +258,7 @@ static unsigned elf_xen_parse_notes(struct elf_binary *elf,
elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
struct elf_dom_parms *parms)
{
- ELF_PTRVAL_CONST_CHAR h;
+ elf_ptrval h;
unsigned char name[32], value[128];
unsigned len;
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index 47957aa..c5e9141 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -118,7 +118,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback
*log_callback,
}
static elf_errorstatus elf_load_image(struct elf_binary *elf,
- ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src,
+ elf_ptrval dst, elf_ptrval src,
uint64_t filesz, uint64_t memsz)
{
elf_memcpy_safe(elf, dst, src, filesz);
@@ -132,7 +132,7 @@ void elf_set_verbose(struct elf_binary *elf)
elf->verbose = 1;
}
-static elf_errorstatus elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID
dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
+static elf_errorstatus elf_load_image(struct elf_binary *elf, elf_ptrval dst,
elf_ptrval src, uint64_t filesz, uint64_t memsz)
{
elf_errorstatus rc;
if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
@@ -187,12 +187,12 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t
pstart)
static void elf_load_bsdsyms(struct elf_binary *elf)
{
- ELF_HANDLE_DECL_NONCONST(elf_ehdr) sym_ehdr;
+ ELF_HANDLE_DECL(elf_ehdr) sym_ehdr;
unsigned long sz;
- ELF_PTRVAL_VOID maxva;
- ELF_PTRVAL_VOID symbase;
- ELF_PTRVAL_VOID symtab_addr;
- ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr;
+ elf_ptrval maxva;
+ elf_ptrval symbase;
+ elf_ptrval symtab_addr;
+ ELF_HANDLE_DECL(elf_shdr) shdr;
unsigned i, type;
if ( !elf->bsd_symtab_pstart )
@@ -226,7 +226,7 @@ do { \
elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(shdr),
ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
sz);
- maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva +
sz);
+ maxva = elf_round_up(elf, (unsigned long)maxva + sz);
for ( i = 0; i < elf_shdr_count(elf); i++ )
{
@@ -242,7 +242,7 @@ do { \
elf_memcpy_safe(elf, maxva, elf_section_start(elf, shdr), sz);
/* Mangled to be based on ELF header location. */
elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
- maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned
long)maxva + sz);
+ maxva = elf_round_up(elf, (unsigned long)maxva + sz);
}
old_shdr_p = ELF_HANDLE_PTRVAL(shdr);
new_shdr_p = old_shdr_p + elf_uval(elf, elf->ehdr, e_shentsize);
@@ -297,7 +297,7 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
{
ELF_HANDLE_DECL(elf_phdr) phdr;
uint64_t i, count, paddr, offset, filesz, memsz;
- ELF_PTRVAL_VOID dest;
+ elf_ptrval dest;
/*
* Let bizarre ELFs write the output image up to twice; this
* calculation is just to ensure our copying loop is no worse than
@@ -334,7 +334,7 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
remain_allow_copy -= memsz;
elf_msg(elf, "%s: phdr %" PRIu64 " at
0x%"ELF_PRPTRVAL" -> 0x%"ELF_PRPTRVAL"\n",
- __func__, i, dest, (ELF_PTRVAL_VOID)(dest + filesz));
+ __func__, i, dest, (elf_ptrval)(dest + filesz));
if ( elf_load_image(elf, dest, ELF_IMAGE_BASE(elf) + offset, filesz,
memsz) != 0 )
return -1;
}
@@ -343,7 +343,7 @@ elf_errorstatus elf_load_binary(struct elf_binary *elf)
return 0;
}
-ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr)
+elf_ptrval elf_get_ptr(struct elf_binary *elf, unsigned long addr)
{
return ELF_REALPTR2PTRVAL(elf->dest_base) + addr - elf->pstart;
}
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index ef13b0d..dae210e 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -171,7 +171,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned
index)
{
uint64_t count = elf_shdr_count(elf);
- ELF_PTRVAL_CONST_VOID ptr;
+ elf_ptrval ptr;
if ( index >= count )
return ELF_INVALID_HANDLE(elf_shdr);
@@ -185,7 +185,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct
elf_binary *elf, unsigned ind
ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned
index)
{
uint64_t count = elf_uval(elf, elf->ehdr, e_phnum);
- ELF_PTRVAL_CONST_VOID ptr;
+ elf_ptrval ptr;
if ( index >= count )
return ELF_INVALID_HANDLE(elf_phdr);
@@ -233,24 +233,24 @@ const char *elf_strfmt(struct elf_binary *elf, elf_ptrval
start)
return str;
}
-ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
+elf_ptrval elf_section_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr)
{
return ELF_IMAGE_BASE(elf) + elf_uval(elf, shdr, sh_offset);
}
-ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr)
+elf_ptrval elf_section_end(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr)
{
return ELF_IMAGE_BASE(elf)
+ elf_uval(elf, shdr, sh_offset) + elf_uval(elf, shdr, sh_size);
}
-ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr)
+elf_ptrval elf_segment_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr)
{
return ELF_IMAGE_BASE(elf)
+ elf_uval(elf, phdr, p_offset);
}
-ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr)
+elf_ptrval elf_segment_end(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr)
{
return ELF_IMAGE_BASE(elf)
+ elf_uval(elf, phdr, p_offset) + elf_uval(elf, phdr, p_filesz);
@@ -258,8 +258,8 @@ ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary
*elf, ELF_HANDLE_DECL(el
ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol)
{
- ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
- ELF_PTRVAL_CONST_VOID end = elf_section_end(elf, elf->sym_tab);
+ elf_ptrval ptr = elf_section_start(elf, elf->sym_tab);
+ elf_ptrval end = elf_section_end(elf, elf->sym_tab);
ELF_HANDLE_DECL(elf_sym) sym;
uint64_t info, name;
const char *sym_name;
@@ -283,7 +283,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary
*elf, const char *sym
ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned
index)
{
- ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
+ elf_ptrval ptr = elf_section_start(elf, elf->sym_tab);
ELF_HANDLE_DECL(elf_sym) sym;
sym = ELF_MAKE_HANDLE(elf_sym, ptr + index * elf_size(elf, sym));
@@ -295,7 +295,7 @@ const char *elf_note_name(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note
return elf_strval(elf, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note));
}
-ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note)
+elf_ptrval elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
@@ -304,7 +304,7 @@ ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_
uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note)
{
- ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
+ elf_ptrval desc = elf_note_desc(elf, note);
unsigned descsz = elf_uval(elf, note, descsz);
switch (descsz)
@@ -322,7 +322,7 @@ uint64_t elf_note_numeric(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note
uint64_t elf_note_numeric_array(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note,
unsigned int unitsz, unsigned int idx)
{
- ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
+ elf_ptrval desc = elf_note_desc(elf, note);
unsigned descsz = elf_uval(elf, note, descsz);
if ( descsz % unitsz || idx >= descsz / unitsz )
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 4ca6b8f..140effb 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -61,13 +61,8 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
/*
* We abstract away the pointerness of these pointers, replacing
* various void*, char* and struct* with the following:
- * PTRVAL A pointer to a byte; one can do pointer arithmetic
+ * elf_ptrval A pointer to a byte; one can do pointer arithmetic
* on this.
- * This replaces variables which were char*,void*
- * and their const versions, so we provide four
- * different obsolete declaration macros:
- * ELF_PTRVAL_{,CONST}{VOID,CHAR}
- * New code can simply use the elf_ptrval typedef.
* HANDLE A pointer to a struct. There is one of these types
* for each pointer type - that is, for each
"structname".
* In the arguments to the various HANDLE macros, structname
@@ -76,8 +71,6 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
* pointers. In the current code attempts to do so will
* compile, but in the next patch this will become a
* compile error.
- * We also provide a second declaration macro for
- * pointers which were to const; this is obsolete.
*/
typedef uintptr_t elf_ptrval;
@@ -85,15 +78,9 @@ typedef uintptr_t elf_ptrval;
#define ELF_REALPTR2PTRVAL(realpointer) ((elf_ptrval)(realpointer))
/* Converts an actual C pointer into a PTRVAL */
-#define ELF_HANDLE_DECL_NONCONST(structname) structname##_handle /*obsolete*/
#define ELF_HANDLE_DECL(structname) structname##_handle
/* Provides a type declaration for a HANDLE. */
-#define ELF_PTRVAL_VOID elf_ptrval /*obsolete*/
-#define ELF_PTRVAL_CHAR elf_ptrval /*obsolete*/
-#define ELF_PTRVAL_CONST_VOID elf_ptrval /*obsolete*/
-#define ELF_PTRVAL_CONST_CHAR elf_ptrval /*obsolete*/
-
#ifdef __XEN__
# define ELF_PRPTRVAL "lu"
/*
@@ -124,17 +111,6 @@ typedef uintptr_t elf_ptrval;
#define ELF_HANDLE_PTRVAL(handleval) ((handleval).ptrval)
/* Converts a HANDLE to a PTRVAL. */
-#define ELF_OBSOLETE_VOIDP_CAST /*empty*/
- /*
- * In some places the old code used to need to
- * - cast away const (the existing code uses const a fair
- * bit but actually sometimes wants to write to its input)
- * from a PTRVAL.
- * - convert an integer representing a pointer to a PTRVAL
- * Nowadays all of these re uintptr_ts so there is no const problem
- * and no need for any casting.
- */
-
#define ELF_UNSAFE_PTR(ptrval) ((void*)(elf_ptrval)(ptrval))
/*
* Turns a PTRVAL into an actual C pointer. Before this is done
@@ -211,7 +187,7 @@ struct elf_binary {
char data;
ELF_HANDLE_DECL(elf_ehdr) ehdr;
- ELF_PTRVAL_CONST_CHAR sec_strtab;
+ elf_ptrval sec_strtab;
ELF_HANDLE_DECL(elf_shdr) sym_tab;
uint64_t sym_strtab;
@@ -289,7 +265,7 @@ struct elf_binary {
* str should be a HANDLE.
*/
-uint64_t elf_access_unsigned(struct elf_binary *elf, ELF_PTRVAL_CONST_VOID ptr,
+uint64_t elf_access_unsigned(struct elf_binary *elf, elf_ptrval ptr,
uint64_t offset, size_t size);
/* Reads a field at arbitrary offset and alignemnt */
@@ -341,17 +317,17 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct
elf_binary *elf, unsigned ind
ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned
index);
const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr); /* might return NULL if inputs are invalid */
-ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
-ELF_PTRVAL_CONST_VOID elf_section_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_shdr) shdr);
+elf_ptrval elf_section_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr);
+elf_ptrval elf_section_end(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr)
shdr);
-ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr);
-ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_phdr) phdr);
+elf_ptrval elf_segment_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr);
+elf_ptrval elf_segment_end(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr)
phdr);
ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char
*symbol);
ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned
index);
const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note); /* may return NULL */
-ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) note);
+elf_ptrval elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note)
note);
uint64_t elf_note_numeric_array(struct elf_binary *, ELF_HANDLE_DECL(elf_note),
unsigned int unitsz, unsigned int idx);
@@ -378,7 +354,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback*,
void elf_parse_binary(struct elf_binary *elf);
elf_errorstatus elf_load_binary(struct elf_binary *elf);
-ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr);
+elf_ptrval elf_get_ptr(struct elf_binary *elf, unsigned long addr);
uint64_t elf_lookup_addr(struct elf_binary *elf, const char *symbol);
void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart); /* private */
@@ -413,9 +389,9 @@ struct xen_elfnote {
struct elf_dom_parms {
/* raw */
- ELF_PTRVAL_CONST_CHAR guest_info;
- ELF_PTRVAL_CONST_VOID elf_note_start;
- ELF_PTRVAL_CONST_VOID elf_note_end;
+ elf_ptrval guest_info;
+ elf_ptrval elf_note_start;
+ elf_ptrval elf_note_end;
struct xen_elfnote elf_notes[XEN_ELFNOTE_MAX + 1];
/* parsed */
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:52 UTC
[PATCH 18/21] libxc: Add range checking to xc_dom_binloader
This is a simple binary image loader with its own metadata format.
However, it is too careless with image-supplied values.
Add the following checks:
* That the image is bigger than the metadata table; otherwise the
pointer arithmetic to calculate the metadata table location may
yield undefined and dangerous values.
* When clamping the end of the region to search, that we do not
calculate pointers before the beginning of the image. The C
specification does not permit this and compilers are becoming ever
more determined to miscompile code when they can "prove" various
falsehoods based on assertions from the C spec.
* That the supplied image is big enough for the text we are allegedly
copying from it. Otherwise we might have a read overrun and copy
the results (perhaps a lot of secret data) into the guest.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: This patch is new in this version of the series.
---
tools/libxc/xc_dom_binloader.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index d2de04c..4e8feae 100644
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -123,9 +123,12 @@ static struct xen_bin_image_table *find_table(struct
xc_dom_image *dom)
uint32_t *probe_ptr;
uint32_t *probe_end;
+ if ( dom->kernel_size < sizeof(*table) )
+ return NULL;
probe_ptr = dom->kernel_blob;
probe_end = dom->kernel_blob + dom->kernel_size - sizeof(*table);
- if ( (void*)probe_end > (dom->kernel_blob + 8192) )
+ if ( dom->kernel_size >= 8192 &&
+ (void*)probe_end > (dom->kernel_blob + 8192) )
probe_end = dom->kernel_blob + 8192;
for ( table = NULL; probe_ptr < probe_end; probe_ptr++ )
@@ -282,6 +285,13 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
return -EINVAL;
}
+ if ( image_size < skip ||
+ image_size - skip < text_size )
+ {
+ DOMPRINTF("%s: image is too small for declared text size",
+ __FUNCTION__);
+ }
+
memcpy(dest, image + skip, text_size);
memset(dest + text_size, 0, bss_size);
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:52 UTC
[PATCH 19/21] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
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.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
v5: This patch is new in this version of the series.
---
tools/libxc/xc_dom_armzimageloader.c | 6 ++++
tools/libxc/xc_dom_binloader.c | 6 ++++
tools/libxc/xc_dom_core.c | 6 ++++
tools/libxc/xc_dom_elfloader.c | 13 ++++++++++
tools/libxc/xc_dom_x86.c | 45 ++++++++++++++++++++++++++++++++++
tools/libxc/xc_domain_restore.c | 27 ++++++++++++++++++++
6 files changed, 103 insertions(+), 0 deletions(-)
diff --git a/tools/libxc/xc_dom_armzimageloader.c
b/tools/libxc/xc_dom_armzimageloader.c
index 74027db..4cbbbab 100644
--- a/tools/libxc/xc_dom_armzimageloader.c
+++ b/tools/libxc/xc_dom_armzimageloader.c
@@ -140,6 +140,12 @@ static int xc_dom_load_zimage_kernel(struct xc_dom_image
*dom)
DOMPRINTF_CALLED(dom->xch);
dst = xc_dom_seg_to_ptr(dom, &dom->kernel_seg);
+ if ( dst == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, &dom->kernel_seg)
=> NULL",
+ __func__);
+ return -1;
+ }
DOMPRINTF("%s: kernel sed %#"PRIx64"-%#"PRIx64,
__func__, dom->kernel_seg.vstart, dom->kernel_seg.vend);
diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index 4e8feae..f8ebed5 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 cf96bfa..21a8e0d 100644
--- a/tools/libxc/xc_dom_core.c
+++ b/tools/libxc/xc_dom_core.c
@@ -870,6 +870,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 1d2727e..ac9bb3b 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -137,6 +137,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/%s:
xc_dom_vaddr_to_ptr(,dom->bsd_symtab_start"
+ " => NULL", __FUNCTION__, load ?
"load" : "parse");
+ return -1;
+ }
elf->caller_xdest_base = hdr_ptr;
elf->caller_xdest_size = allow_size;
hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
@@ -383,6 +389,13 @@ static elf_errorstatus xc_dom_load_elf_kernel(struct
xc_dom_image *dom)
elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg,
&pages);
elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom);
+ if ( elf->dest_base == NULL )
+ {
+ DOMPRINTF("%s: xc_dom_vaddr_to_ptr(,dom->kernel_seg)"
+ " => NULL", __FUNCTION__);
+ return -1;
+ }
+
rc = elf_load_binary(elf);
if ( rc < 0 )
{
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index f1be43b..8b6191d 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -223,6 +223,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. "
@@ -266,6 +272,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 )
@@ -274,6 +282,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;
@@ -284,6 +294,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;
@@ -310,6 +322,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
@@ -347,6 +364,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 )
{
@@ -354,6 +374,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;
@@ -364,6 +386,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;
@@ -376,6 +400,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;
@@ -396,6 +422,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
@@ -413,6 +444,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");
@@ -437,6 +470,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'';
@@ -477,6 +516,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 a15f86a..c7835ff 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1638,6 +1638,12 @@ int xc_domain_restore(xc_interface *xch, int io_fd,
uint32_t dom,
mfn = ctx->p2m[pfn];
buf = xc_map_foreign_range(xch, dom, PAGE_SIZE,
PROT_READ | PROT_WRITE, mfn);
+ if ( buf == NULL )
+ {
+ ERROR("xc_map_foreign_range for generation
id"
+ " buffer failed");
+ goto out;
+ }
generationid = *(unsigned long long *)(buf + offset);
*(unsigned long long *)(buf + offset) = generationid + 1;
@@ -1794,6 +1800,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];
@@ -1820,6 +1831,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];
@@ -1996,6 +2012,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);
@@ -2143,6 +2165,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
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@eu.citrix.com>
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 | 8 ++++++++
tools/libxc/xc_linux_osdep.c | 4 ++++
tools/libxc/xc_private.c | 2 ++
tools/libxc/xenctrl.h | 2 +-
9 files changed, 43 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 ) {
+ 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..66ffc41 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1532,8 +1532,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_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
--
1.7.2.5
Ian Jackson
2013-Jun-06 18:52 UTC
[PATCH 21/21] libxc: range checks in xc_dom_p2m_host and _guest
These functions take guest pfns and look them up in the p2m. They did
no range checking.
However, some callers, notably xc_dom_boot.c:setup_hypercall_page want
to pass untrusted guest-supplied value(s). It is most convenient to
detect this here and return INVALID_MFN.
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_dom.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index 567913f..239ef33 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -341,6 +341,8 @@ static inline xen_pfn_t xc_dom_p2m_host(struct xc_dom_image
*dom, xen_pfn_t pfn)
{
if (dom->shadow_enabled)
return pfn;
+ if (pfn >= dom->rambase_pfn + dom->total_pages)
+ return INVALID_MFN;
return dom->p2m_host[pfn - dom->rambase_pfn];
}
@@ -349,6 +351,8 @@ static inline xen_pfn_t xc_dom_p2m_guest(struct xc_dom_image
*dom,
{
if (xc_dom_feature_translated(dom))
return pfn;
+ if (pfn >= dom->rambase_pfn + dom->total_pages)
+ return INVALID_MFN;
return dom->p2m_host[pfn - dom->rambase_pfn];
}
--
1.7.2.5
Ian Jackson writes ("[PATCH v5 00/21] XSA55 libelf fixes for
unstable"):> This is version 5 of my to try to fix libelf and the domain loader.
This is available via git in the same place as before:
http://xenbits.xen.org/gitweb/?p=people/iwj/xen-unstable.git;a=summary
git://xenbits.xen.org/people/iwj/xen-unstable.git
in the commits
xsa55-unstable-base-rebasing..xsa55-unstable-rebasing
Ian.
Andrew Cooper
2013-Jun-06 19:47 UTC
Re: [PATCH 20/21] libxc: check return values from malloc
On 06/06/13 19:52, 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@eu.citrix.com> > > 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 | 8 ++++++++ > tools/libxc/xc_linux_osdep.c | 4 ++++ > tools/libxc/xc_private.c | 2 ++ > tools/libxc/xenctrl.h | 2 +- > 9 files changed, 43 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 ) { > + 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..66ffc41 100644 > --- a/tools/libxc/xc_domain_restore.c > +++ b/tools/libxc/xc_domain_restore.c > @@ -1532,8 +1532,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; > + } > + }Can you not merge this NULL test into the if statement immediatly below which does all the other NULL pointer checks? something like (!ctx->hvm && ctx->superpages && ctx->p2m_saved_batch == NULL ~Andrew> > 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_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 >
On Fri, Jun 7, 2013 at 6:51 AM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:> This is version 5 of my to try to fix libelf and the domain loader.Might want to add a proper length check to xc_dom_check_gzip like this too while you''re at it: From 5dc67065fdc81408e2c9b49ab26ac2822437bd2f Mon Sep 17 00:00:00 2001 From: Matthew Daley <mattjd@gmail.com> Date: Fri, 7 Jun 2013 20:08:49 +1200 Subject: [PATCH] libxc: check blob size before proceeding in xc_dom_check_gzip Signed-off-by: Matthew Daley <mattjd@gmail.com> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c index 2a9c5a2..525b364 100644 --- a/tools/libxc/xc_dom_core.c +++ b/tools/libxc/xc_dom_core.c @@ -278,6 +278,10 @@ size_t xc_dom_check_gzip(xc_interface *xch, void *blob, size_t ziplen) unsigned char *gzlen; size_t unziplen; + if ( ziplen < 6 ) + /* too small */ + return 0; + if ( strncmp(blob, "\037\213", 2) ) /* not gzipped */ return 0; -- 1.7.10.4
Andrew Cooper
2013-Jun-07 08:59 UTC
Re: [PATCH 21/21] libxc: range checks in xc_dom_p2m_host and _guest
On 06/06/13 19:52, Ian Jackson wrote:> These functions take guest pfns and look them up in the p2m. They did > no range checking. > > However, some callers, notably xc_dom_boot.c:setup_hypercall_page want > to pass untrusted guest-supplied value(s). It is most convenient to > detect this here and return INVALID_MFN. > > 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_dom.h | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h > index 567913f..239ef33 100644 > --- a/tools/libxc/xc_dom.h > +++ b/tools/libxc/xc_dom.h > @@ -341,6 +341,8 @@ static inline xen_pfn_t xc_dom_p2m_host(struct xc_dom_image *dom, xen_pfn_t pfn) > { > if (dom->shadow_enabled) > return pfn; > + if (pfn >= dom->rambase_pfn + dom->total_pages) > + return INVALID_MFN;Need to check whether pfn < dom->rambase_pfn otherwise you can just as easily underflow the array. ~Andrew> return dom->p2m_host[pfn - dom->rambase_pfn]; > } > > @@ -349,6 +351,8 @@ static inline xen_pfn_t xc_dom_p2m_guest(struct xc_dom_image *dom, > { > if (xc_dom_feature_translated(dom)) > return pfn; > + if (pfn >= dom->rambase_pfn + dom->total_pages) > + return INVALID_MFN; > return dom->p2m_host[pfn - dom->rambase_pfn]; > } >
Ian Jackson
2013-Jun-07 10:45 UTC
Re: [PATCH 20/21] libxc: check return values from malloc
Andrew Cooper writes ("Re: [PATCH 20/21] libxc: check return values from
malloc"):> On 06/06/13 19:52, Ian Jackson wrote:
> > 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;
> > + }
> > + }
>
> Can you not merge this NULL test into the if statement immediatly below
> which does all the other NULL pointer checks?
>
> something like (!ctx->hvm && ctx->superpages &&
ctx->p2m_saved_batch == NULL
I thought it better to duplicate the formulaic and entirely obvious
error handling code than the non-obvious and semantically relevant
test for whether to allocate.
Ian.
Matthew Daley
2013-Jun-07 10:59 UTC
Re: [PATCH 18/21] libxc: Add range checking to xc_dom_binloader
On Fri, Jun 7, 2013 at 6:52 AM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:> This is a simple binary image loader with its own metadata format. > However, it is too careless with image-supplied values. > > Add the following checks: > > * That the image is bigger than the metadata table; otherwise the > pointer arithmetic to calculate the metadata table location may > yield undefined and dangerous values. > > * When clamping the end of the region to search, that we do not > calculate pointers before the beginning of the image.Don''t you mean after the end of the image? I can''t reconcile this bit with the actual patch.> The C > specification does not permit this and compilers are becoming ever > more determined to miscompile code when they can "prove" various > falsehoods based on assertions from the C spec. > > * That the supplied image is big enough for the text we are allegedly > copying from it. Otherwise we might have a read overrun and copy > the results (perhaps a lot of secret data) into the guest. > > This is part of the fix to a security issue, XSA-55. > > Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> > > v5: This patch is new in this version of the series. > --- > tools/libxc/xc_dom_binloader.c | 12 +++++++++++- > 1 files changed, 11 insertions(+), 1 deletions(-) > > diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c > index d2de04c..4e8feae 100644 > --- a/tools/libxc/xc_dom_binloader.c > +++ b/tools/libxc/xc_dom_binloader.c > @@ -123,9 +123,12 @@ static struct xen_bin_image_table *find_table(struct xc_dom_image *dom) > uint32_t *probe_ptr; > uint32_t *probe_end; > > + if ( dom->kernel_size < sizeof(*table) ) > + return NULL; > probe_ptr = dom->kernel_blob; > probe_end = dom->kernel_blob + dom->kernel_size - sizeof(*table); > - if ( (void*)probe_end > (dom->kernel_blob + 8192) ) > + if ( dom->kernel_size >= 8192 && > + (void*)probe_end > (dom->kernel_blob + 8192) ) > probe_end = dom->kernel_blob + 8192; > > for ( table = NULL; probe_ptr < probe_end; probe_ptr++ ) > @@ -282,6 +285,13 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom) > return -EINVAL; > } > > + if ( image_size < skip || > + image_size - skip < text_size ) > + { > + DOMPRINTF("%s: image is too small for declared text size", > + __FUNCTION__);return -EINVAL (or similar) is needed here.> + } > + > memcpy(dest, image + skip, text_size); > memset(dest + text_size, 0, bss_size); > > -- > 1.7.2.5 >
Matthew Daley writes ("Re: [PATCH v5 00/21] XSA55 libelf fixes for
unstable"):> Might want to add a proper length check to xc_dom_check_gzip like this
> too while you''re at it:
*sigh* Yes, you are right.
Thanks, pushed that onto the end of my series.
Ian.
Ian Jackson
2013-Jun-07 14:38 UTC
Re: [PATCH 21/21] libxc: range checks in xc_dom_p2m_host and _guest
Andrew Cooper writes ("Re: [PATCH 21/21] libxc: range checks in
xc_dom_p2m_host and _guest"):> On 06/06/13 19:52, Ian Jackson wrote:
> > + if (pfn >= dom->rambase_pfn + dom->total_pages)
> > + return INVALID_MFN;
>
> Need to check whether pfn < dom->rambase_pfn otherwise you can just
as
> easily underflow the array.
Duh, yes, thanks.
Ian.
Ian Jackson
2013-Jun-07 14:53 UTC
Re: [PATCH 18/21] libxc: Add range checking to xc_dom_binloader
Matthew Daley writes ("Re: [PATCH 18/21] libxc: Add range checking to
xc_dom_binloader"):> On Fri, Jun 7, 2013 at 6:52 AM, Ian Jackson
<ian.jackson@eu.citrix.com> wrote:
> > This is a simple binary image loader with its own metadata format.
> > However, it is too careless with image-supplied values.
> >
> > Add the following checks:
> >
> > * That the image is bigger than the metadata table; otherwise the
> > pointer arithmetic to calculate the metadata table location may
> > yield undefined and dangerous values.
> >
> > * When clamping the end of the region to search, that we do not
> > calculate pointers before the beginning of the image.
>
> Don''t you mean after the end of the image? I can''t
reconcile this bit
> with the actual patch.
You''re right.
> > + if ( image_size < skip ||
> > + image_size - skip < text_size )
> > + {
> > + DOMPRINTF("%s: image is too small for declared text
size",
> > + __FUNCTION__);
>
> return -EINVAL (or similar) is needed here.
Oops.
Ian.