This is version 3.1 of my series to try to fix libelf. (v3 was
published only via xsa.git.)
These are all in xsa.git. Additionally, I have prepared backports for
Xen 4.2 and 4.1 which are there too. I''m going to send all three off
to osstest for another once-over.
Review, please, particularly of the ones listed with only zero or one
ack/review.
Thanks,
Ian.
Here is a summary of the series:
a 01/16 libelf: abolish libelf-relocate.c
A 02/16 libxc: introduce xc_dom_seg_to_ptr_pages
A 03/16 libelf: add `struct elf_binary*'' parameter to elf_load_image
a 04/16 libelf: abolish elf_sval and elf_access_signed
A 05/16 libelf: move include of <asm/guest_access.h> to top of file
a 06/16 libelf/xc_dom_load_elf_symtab: Do not use "syms"
uninitialised
* 07/16 libxl: introduce macros for memory access and pointer handling
a 08/16 tools/xcutils/readnotes: adjust print_l1_mfn_valid_note
A 09/16 libelf: check nul-terminated strings properly
* 10/16 libxl: check all pointer accesses
A 11/16 libxl: Check pointer references in elf_is_elfbinary
* 12/16 libelf: Make all callers call elf_check_broken
+ 13/16 libelf: use C99 bool for booleans
/ 14/16 libelf: use only unsigned integers
- 15/16 libelf: check loops for running away
a 16/16 libelf: abolish obsolete macros
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).
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 verified to terminate.
* 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).
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.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
tools/libxc/xc_dom.h | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index ac36600..6c31440 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -294,16 +294,24 @@ 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;
+ if (pages_out) *pages_out = pages;
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,
xen_vaddr_t vaddr)
{
--
1.7.2.5
Ian Jackson
2013-Jun-03 15:41 UTC
[PATCH 03/16] 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.
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-03 15:41 UTC
[PATCH 04/16] libelf: abolish elf_sval and elf_access_signed
These are not used anywhere.
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-03 15:41 UTC
[PATCH 05/16] 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.
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-03 15:41 UTC
[PATCH 06/16] 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.
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 6583859..d2fc977 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -141,7 +141,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-03 15:41 UTC
[PATCH 07/16] 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.
-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<-
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>
v3.1:
Change elf_store_field to verify correctly on 32-bit i386.
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.
---
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 | 82 +++++++++-------
xen/common/libelf/libelf-tools.c | 94 +++++++++---------
xen/include/xen/libelf.h | 188 +++++++++++++++++++++++++++++++-----
7 files changed, 311 insertions(+), 162 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index d2fc977..9714b1e 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;
@@ -144,11 +144,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) )
{
@@ -186,7 +186,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 )
{
@@ -202,9 +202,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;
}
}
@@ -213,9 +213,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++;
@@ -227,7 +227,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);
}
@@ -235,9 +235,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 )
@@ -272,7 +272,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..5e0ef66 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",
+ elf_msg(elf, "%s: phdr %" PRIu64 " at
0x%"ELF_PRPTRVAL" -> 0x%"ELF_PRPTRVAL"\n",
__func__, i, dest, dest + filesz);
- if ( elf_load_image(elf, dest, elf->image + offset, filesz, memsz)
!= 0 )
+ 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..62a1cb0 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*)
+ /*
+ * 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-03 15:41 UTC
[PATCH 08/16] 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-03 15:41 UTC
[PATCH 09/16] 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.
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 62a1cb0..72b9eb0 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.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
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 | 103 +++++++++++++++++++-
xen/include/xen/libelf.h | 184 ++++++++++++++++++++++++------------
8 files changed, 296 insertions(+), 84 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 9714b1e..81b7cdd 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -118,6 +118,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
ELF_PTRVAL_CHAR hdr;
size_t size;
int h, count, type, i, tables = 0;
+ unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
if ( elf_swap(elf) )
{
@@ -128,19 +129,30 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
if ( load )
{
+ char *hdr_ptr;
+
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);
- *(int *)hdr = size - sizeof(int);
+ hdr_ptr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start);
+ elf->caller_xdest_base = hdr_ptr;
+ elf->caller_xdest_size = page_size -
+ (dom->bsd_symtab_start & (page_size-1));
+ 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);
}
@@ -168,9 +180,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);
@@ -309,8 +344,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 5e0ef66..0e43475 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..4e9ac21 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -20,28 +20,97 @@
/* ------------------------------------------------------------------------ */
-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.
+ */
+{
+ elf_ptrval regionp = (elf_ptrval)region;
+
+ if ( (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 +191,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 72b9eb0..22d966d 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,51 +68,55 @@ 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*/
+
+#define ELF_PRPTRVAL PRIuPTR
+ /* 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*)
+#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))
@@ -122,23 +127,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 +189,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 +197,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 +214,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 +247,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 +279,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 +348,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 +367,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 +444,33 @@ 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_size >= amount )
+ {
+ elf->dest_base += amount;
+ elf->dest_size -= amount;
+ }
+ else
+ {
+ elf->dest_size = 0;
+ }
+}
#endif /* __XEN_LIBELF_H__ */
--
1.7.2.5
Ian Jackson
2013-Jun-03 15:41 UTC
[PATCH 11/16] 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.
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 81b7cdd..6201fd4 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 0e43475..471f7a7 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 4e9ac21..1d5963a 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -329,11 +329,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 22d966d..f3491cc 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -341,7 +341,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-03 15:41 UTC
[PATCH 12/16] 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".
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
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 6201fd4..14f1df4 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -275,6 +275,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__);
@@ -311,19 +318,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 */
@@ -337,7 +348,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..b868fba 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.)
Signed-off-by: Ian Jackson <ian.jackson@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 14f1df4..923e6f4 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 471f7a7..2e8091f 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 1d5963a..f96ac7a 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>
@@ -50,7 +50,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) )
@@ -89,7 +89,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;
@@ -329,7 +329,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;
@@ -339,7 +339,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 f3491cc..bf21ed9 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
@@ -228,7 +230,7 @@ struct elf_binary {
elf_log_callback *log_callback;
void *log_caller_data;
#endif
- int verbose;
+ bool verbose;
const char *broken;
};
@@ -292,8 +294,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) \
({ \
@@ -341,8 +343,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 */
@@ -356,7 +358,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);
@@ -408,7 +410,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").
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>
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 | 50 ++++++++++++++++++------------------
xen/common/libelf/libelf-loader.c | 20 +++++++-------
xen/common/libelf/libelf-tools.c | 24 ++++++++--------
xen/include/xen/libelf.h | 21 ++++++++------
9 files changed, 103 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 6c31440..d4b4f0d 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 923e6f4..83e16ef 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;
unsigned int page_size = XC_DOM_PAGE_SIZE(dom);
if ( elf_swap(elf) )
@@ -139,13 +139,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image
*dom,
elf->caller_xdest_size = page_size -
(dom->bsd_symtab_start & (page_size-1));
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 )
@@ -156,15 +156,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;
@@ -173,22 +173,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;
/*
@@ -208,7 +208,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));
@@ -254,7 +254,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);
@@ -293,10 +293,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 )
@@ -357,10 +361,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 b868fba..9be1594 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..a9a5f41 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,12 @@ int elf_xen_parse_note(struct elf_binary *elf,
return 0;
}
-static int elf_xen_parse_notes(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)
{
- int xen_elfnotes = 0;
+ unsigned xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
const char *note_name;
@@ -237,7 +237,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 ~0U;
xen_elfnotes++;
}
return xen_elfnotes;
@@ -246,12 +246,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 +334,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 +378,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 +464,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 +495,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 == ~0U )
return -1;
- xen_elfnotes += rc;
+ xen_elfnotes += more_notes;
}
/*
@@ -518,17 +518,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 == ~0U )
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 2e8091f..bcdd3d2 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 f96ac7a..b47a9ca 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -119,19 +119,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;
+ unsigned 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);
}
@@ -141,7 +141,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++ )
{
@@ -153,7 +153,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;
@@ -167,7 +167,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;
@@ -261,7 +261,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;
@@ -277,7 +277,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;
}
@@ -285,7 +285,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)
{
@@ -303,7 +303,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;
@@ -321,8 +321,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 bf21ed9..d53497f 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__
@@ -319,12 +322,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);
@@ -334,7 +337,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);
@@ -349,7 +352,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.
@@ -362,7 +365,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);
@@ -375,7 +378,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 */
@@ -409,7 +412,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.
* 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.
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.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
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 | 3 +++
xen/common/libelf/libelf-dominfo.c | 14 ++++++++++++++
xen/common/libelf/libelf-loader.c | 27 +++++++++++++++++++++++++--
xen/common/libelf/libelf-tools.c | 11 ++++++++++-
4 files changed, 52 insertions(+), 3 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 83e16ef..b6671a1 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -222,6 +222,9 @@ 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);
+ 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 )
{
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index a9a5f41..289132e 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -485,6 +485,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;
@@ -515,6 +522,10 @@ 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;
@@ -552,6 +563,9 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf,
elf_xen_parse_guest_info(elf, parms);
break;
}
+ if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
+ /* input has an insane section header count field */
+ break;
}
}
diff --git a/xen/common/libelf/libelf-loader.c
b/xen/common/libelf/libelf-loader.c
index bcdd3d2..26ca839 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);
@@ -283,6 +303,9 @@ elf_errorstatus elf_load_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);
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index b47a9ca..309a134 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -145,6 +145,9 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary
*elf, const char *n
for ( i = 0; i < count; i++ )
{
+ if (!elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1))
+ /* input has an insane section header count field */
+ break;
shdr = elf_shdr_by_index(elf, i);
sname = elf_section_name(elf, shdr);
if ( sname && !strcmp(sname, name) )
@@ -324,7 +327,13 @@ 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))
+ ptrval = ELF_MAX_PTRVAL; /* terminate caller''s loop */
+
+ return ELF_MAKE_HANDLE(elf_note, ptrval);
}
/* ------------------------------------------------------------------------ */
--
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.)
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 | 22 ++++++++--------
xen/common/libelf/libelf-tools.c | 24 +++++++++---------
xen/include/xen/libelf.h | 48 +++++++++---------------------------
6 files changed, 43 insertions(+), 67 deletions(-)
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index b6671a1..1fa2484 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -113,9 +113,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 int page_size = XC_DOM_PAGE_SIZE(dom);
@@ -221,7 +221,7 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct
xc_dom_image *dom,
count = elf_shdr_count(&syms);
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;
@@ -265,7 +265,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 9be1594..4ff7085 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 289132e..938d721 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -218,8 +218,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 xen_elfnotes = 0;
ELF_HANDLE_DECL(elf_note) note;
@@ -249,7 +249,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 26ca839..b850202 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;
count = elf_uval(elf, elf->ehdr, e_phnum);
for ( i = 0; i < count; i++ )
@@ -323,7 +323,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 309a134..88470cc 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -159,7 +159,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);
@@ -173,7 +173,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);
@@ -216,24 +216,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);
@@ -241,8 +241,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;
@@ -266,7 +266,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));
@@ -278,7 +278,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;
@@ -287,7 +287,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)
@@ -305,7 +305,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 d53497f..e8225bf 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -58,13 +58,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
@@ -73,8 +68,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;
@@ -82,15 +75,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*/
-
#define ELF_PRPTRVAL PRIuPTR
/* printf format a la PRId... for a PTRVAL */
@@ -113,17 +100,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*)(ptrval))
/*
* Turns a PTRVAL into an actual C pointer. Before this is done
@@ -200,7 +176,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;
@@ -278,7 +254,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 */
@@ -330,17 +306,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);
@@ -367,7 +343,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 */
@@ -402,9 +378,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 writes ("[PATCH v3.1 00/16] XSA55 libelf fixes for
unstable"):> This is version 3.1 of my series to try to fix libelf. (v3 was
> published only via xsa.git.)
Needless to say this was not meant to go to xen-devel. I''m very sorry
and feel like a right idiot.
My mistake was that I used the wrong saved rune to send the patch
series. Given the complexity and scope of the work slips are
inevitable :-/. I think at the very least that I need to fix my own
workflow so that a simple slip doesn''t have this kind of consequence.
Ian.
Ian Jackson writes ("[PATCH 14/16] libelf: use only unsigned
integers"):> index f96ac7a..b47a9ca 100644
> --- a/xen/common/libelf/libelf-tools.c
> +++ b/xen/common/libelf/libelf-tools.c
> @@ -119,19 +119,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;
> + unsigned elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
>
> return (addr + elf_round) & ~elf_round;
> }
It has been pointed out to me that this is wrong and truncates the
value to 32-bit. The correct fix is to change elf_round to uint64_t.
This shows a hole in the argument for the safety of this patch.
Ian.
Matthew Daley
2013-Jun-04 14:49 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:> @@ -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 ||Should be break in this case, not continue, otherwise a invalid large elf note section will make the loop run for a long time.> @@ -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;Similarly, should be break, not continue. (For my reference: this is crashsig f08825f)
On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson <ian.jackson@eu.citrix.com> wrote:> +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. > + */ > +{ > + elf_ptrval regionp = (elf_ptrval)region; > + > + if ( (ptrval < regionp) || /* start is before region */ > + (ptrval > regionp + regionsize) || /* start is after region */Should be >=, not >, FWIW.
Matthew Daley writes ("Re: [PATCH 10/16] libelf: check all pointer
accesses"):> On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson
<ian.jackson@eu.citrix.com> wrote:
> > +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.
> > + */
> > +{
> > + elf_ptrval regionp = (elf_ptrval)region;
> > +
> > + if ( (ptrval < regionp) || /* start is before
region */
> > + (ptrval > regionp + regionsize) || /* start is after
region */
>
> Should be >=, not >, FWIW.
I think this is correct.
If size==0, ptrval == regionp+regionsize is OK.
If size>0 and ptrval==regionp+regionsize then
`regionsize - (ptrval - regionp)'' is 0 and we fail.
Ian.
Ian Jackson
2013-Jun-04 15:15 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
Matthew Daley writes ("Re: [PATCH 09/16] libelf: check nul-terminated
strings properly"):> On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson
<ian.jackson@eu.citrix.com> wrote:
> > @@ -103,10 +103,13 @@ static int print_notes(struct elf_binary *elf,
ELF_HANDLE_DECL(elf_note) start,
> > - if (0 != strcmp(elf_note_name(elf, note),
"Xen"))
> > + this_note_name = elf_note_name(elf, note);
> > + if (NULL == this_note_name ||
>
> Should be break in this case, not continue, otherwise a invalid large
> elf note section will make the loop run for a long time.
I think the real problem here is the nested loop. Perhaps we should
impose a smallish limit on the size of strings.
I''ve taken the view that looping once per byte of the input image is
fine. But I think I this loop (and others like it) can be made to
loop O(size_of_input_image^2) times.
Changing the continue to break doesn''t help because if the attacker
puts a nul byte at the end of their image they get to execute the same
amount of loop.
Ian.
Matthew Daley
2013-Jun-04 15:27 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
On Wed, Jun 5, 2013 at 3:15 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> Matthew Daley writes ("Re: [PATCH 09/16] libelf: check nul-terminated strings properly"): >> On Tue, Jun 4, 2013 at 3:41 AM, Ian Jackson <ian.jackson@eu.citrix.com> wrote: >> > @@ -103,10 +103,13 @@ static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start, >> > - if (0 != strcmp(elf_note_name(elf, note), "Xen")) >> > + this_note_name = elf_note_name(elf, note); >> > + if (NULL == this_note_name || >> >> Should be break in this case, not continue, otherwise a invalid large >> elf note section will make the loop run for a long time. > > I think the real problem here is the nested loop. Perhaps we should > impose a smallish limit on the size of strings. > > I''ve taken the view that looping once per byte of the input image is > fine. But I think I this loop (and others like it) can be made to > loop O(size_of_input_image^2) times. > > Changing the continue to break doesn''t help because if the attacker > puts a nul byte at the end of their image they get to execute the same > amount of loop. > > Ian.I think I screwed up my explanation. I meant, if the user-supplied end address of the note section is way off base - outside of the actual image - elf_note_next will eventually read out-of-range 0s for the two note sizes, and return a out-of-range handle note just past the last one. elf_note_name will return NULL on this out-of-range note handle, and the loop continues - maybe for up to ~0ul / 12 ''notes''. I agree with the O(n^2) problem too, though.
Ian Jackson
2013-Jun-04 17:13 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
Matthew Daley writes ("Re: [PATCH 09/16] libelf: check nul-terminated
strings properly"):> I think I screwed up my explanation. I meant, if the user-supplied end
> address of the note section is way off base - outside of the actual
> image - elf_note_next will eventually read out-of-range 0s for the two
> note sizes, and return a out-of-range handle note just past the last
> one. elf_note_name will return NULL on this out-of-range note handle,
> and the loop continues - maybe for up to ~0ul / 12
''notes''.
Oh I see.
I think I should fix this by having elf_note_next check that the
returned value is in range and return ELF_MAX_PTRVAL if it isn''t.
> I agree with the O(n^2) problem too, though.
This was a bit of a can of worms...
Ian.
Matthew Daley
2013-Jun-05 02:08 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
On Wed, Jun 5, 2013 at 5:13 AM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> Matthew Daley writes ("Re: [PATCH 09/16] libelf: check nul-terminated strings properly"): >> I think I screwed up my explanation. I meant, if the user-supplied end >> address of the note section is way off base - outside of the actual >> image - elf_note_next will eventually read out-of-range 0s for the two >> note sizes, and return a out-of-range handle note just past the last >> one. elf_note_name will return NULL on this out-of-range note handle, >> and the loop continues - maybe for up to ~0ul / 12 ''notes''. > > Oh I see. > > I think I should fix this by having elf_note_next check that the > returned value is in range and return ELF_MAX_PTRVAL if it isn''t.Indeed, this has been fixed in v4.
Ian Campbell
2013-Jun-05 07:24 UTC
Re: [PATCH 09/16] libelf: check nul-terminated strings properly
On Tue, 2013-06-04 at 18:13 +0100, Ian Jackson wrote:> > I agree with the O(n^2) problem too, though. > > This was a bit of a can of worms...Would it be useful to get the initial set of patches dealing with the wild pointers etc into shape and into the trees (and backported) and then deal with the algorithmic complexities next? By bundling more issues into the existing series I worry that we are making it harder to make progress overall. Ian.