From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 11/22] libelf: check all pointer accesses Date: Tue, 11 Jun 2013 00:38:16 +0100 Message-ID: <51B66368.6010602@citrix.com> References: <1370629642-6990-1-git-send-email-ian.jackson@eu.citrix.com> <1370629642-6990-12-git-send-email-ian.jackson@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1370629642-6990-12-git-send-email-ian.jackson@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: "xen-devel@lists.xensource.com" , "mattjd@gmail.com" , "security@xen.org" List-Id: xen-devel@lists.xenproject.org On 07/06/13 19:27, Ian Jackson wrote: > We change the ELF_PTRVAL and ELF_HANDLE types and associated macros: > > * PTRVAL becomes a uintptr_t, for which we provide a typedef > elf_ptrval. This means no arithmetic done on it can overflow so > the compiler cannot do any malicious invalid pointer arithmetic > "optimisations". It also means that any places where we > dereference one of these pointers without using the appropriate > macros or functions become a compilation error. > > So we can be sure that we won't miss any memory accesses. > > All the PTRVAL variables were previously void* or char*, so > the actual address calculations are unchanged. > > * ELF_HANDLE becomes a union, one half of which keeps the pointer > value and the other half of which is just there to record the > type. > > The new type is not a pointer type so there can be no address > calculations on it whose meaning would change. Every assignment or > access has to go through one of our macros. > > * The distinction between const and non-const pointers and char*s > and void*s in libelf goes away. This was not important (and > anyway libelf tended to cast away const in various places). > > * The fields elf->image and elf->dest are renamed. That proves > that we haven't missed any unchecked uses of these actual > pointer values. > > * The caller may fill in elf->caller_xdest_base and _size to > specify another range of memory which is safe for libelf to > access, besides the input and output images. > > * When accesses fail due to being out of range, we mark the elf > "broken". This will be checked and used for diagnostics in > a following patch. > > We do not check for write accesses to the input image. This is > because libelf actually does this in a number of places. So we > simply permit that. > > * Each caller of libelf which used to set dest now sets > dest_base and dest_size. > > * In xc_dom_load_elf_symtab we provide a new actual-pointer > value hdr_ptr which we get from mapping the guest's kernel > area and use (checking carefully) as the caller_xdest area. > > * The STAR(h) macro in libelf-dominfo.c now uses elf_access_unsigned. > > * elf-init uses the new elf_uval_3264 accessor to access the 32-bit > fields, rather than an unchecked field access (ie, unchecked > pointer access). > > * elf_uval has been reworked to use elf_uval_3264. Both of these > macros are essentially new in this patch (although they are derived > from the old elf_uval) and need careful review. > > * ELF_ADVANCE_DEST is now safe in the sense that you can use it to > chop parts off the front of the dest area but if you chop more than > is available, the dest area is simply set to be empty, preventing > future accesses. > > * We introduce some #defines for memcpy, memset, memmove and strcpy: > - We provide elf_memcpy_safe and elf_memset_safe which take > PTRVALs and do checking on the supplied pointers. > - Users inside libelf must all be changed to either > elf_mem*_unchecked (which are just like mem*), or > elf_mem*_safe (which take PTRVALs) and are checked. Any > unchanged call sites become compilation errors. > > * We do _not_ at this time fix elf_access_unsigned so that it doesn't > make unaligned accesses. We hope that unaligned accesses are OK on > every supported architecture. But it does check the supplied > pointer for validity. > > This is part of the fix to a security issue, XSA-55. > > Signed-off-by: Ian Jackson > > v5: Use allow_size value from xc_dom_vaddr_to_ptr to set xdest_size > correctly. > If ELF_ADVANCE_DEST advances past the end, mark the elf broken. > Always regard NULL allowable region pointers (e.g. dest_base) > as invalid (since NULL pointers don't point anywhere). > > v4: Fix ELF_UNSAFE_PTR to work on 32-bit even when provided 64-bit > values. > Fix xc_dom_load_elf_symtab not to call XC_DOM_PAGE_SIZE > unnecessarily if load is false. This was a regression. > > v3.1: > Introduce a change to elf_store_field to undo the effects of > the v3.1 change to the previous patch (the definition there > is not compatible with the new types). > > v3: Fix a whitespace error. > > v2 was Acked-by: Ian Campbell > > v2: BUGFIX: elf_strval: Fix loop termination condition to actually work. > BUGFIX: elf_strval: Fix return value to not always be totally wild. > BUGFIX: xc_dom_load_elf_symtab: do proper check for small header size. > xc_dom_load_elf_symtab: narrow scope of `hdr_ptr'. > xc_dom_load_elf_symtab: split out uninit'd symtab.class ref fix. > More comments on the lifetime/validity of elf-> dest ptrs etc. > libelf.h: write "obsolete" out in full > libelf.h: rename "dontuse" to "typeonly" and add doc comment > elf_ptrval_in_range: Document trustedness of arguments. > Style and commit message fixes. > --- > tools/libxc/xc_dom_elfloader.c | 49 ++++++++-- > tools/libxc/xc_hvm_build_x86.c | 10 +- > xen/arch/x86/domain_build.c | 3 +- > xen/common/libelf/libelf-dominfo.c | 2 +- > xen/common/libelf/libelf-loader.c | 16 ++-- > xen/common/libelf/libelf-private.h | 13 +++ > xen/common/libelf/libelf-tools.c | 106 ++++++++++++++++++- > xen/include/xen/libelf.h | 199 +++++++++++++++++++++++++----------- > 8 files changed, 312 insertions(+), 86 deletions(-) > > diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c > index b8089bc..c038d1c 100644 > --- a/tools/libxc/xc_dom_elfloader.c > +++ b/tools/libxc/xc_dom_elfloader.c > @@ -128,20 +128,30 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom, > > if ( load ) > { > - size_t allow_size; /* will be used in a forthcoming XSA-55 patch */ > + char *hdr_ptr; > + size_t allow_size; > + > if ( !dom->bsd_symtab_start ) > return 0; > size = dom->kernel_seg.vend - dom->bsd_symtab_start; > - hdr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start, &allow_size); > - *(int *)hdr = size - sizeof(int); > + hdr_ptr = xc_dom_vaddr_to_ptr(dom, dom->bsd_symtab_start, &allow_size); > + elf->caller_xdest_base = hdr_ptr; > + elf->caller_xdest_size = allow_size; > + hdr = ELF_REALPTR2PTRVAL(hdr_ptr); > + elf_store_val(elf, int, hdr, size - sizeof(int)); > } Is it not sensible to move the later "check failure of xc_dom_*_to_ptr()" to here? It would certainly fall into the category of "check all pointer accesses". > else > { > + char *hdr_ptr; > + > size = sizeof(int) + elf_size(elf, elf->ehdr) + > elf_shdr_count(elf) * elf_size(elf, shdr); > - hdr = xc_dom_malloc(dom, size); > - if ( hdr == NULL ) > + hdr_ptr = xc_dom_malloc(dom, size); > + if ( hdr_ptr == NULL ) > return 0; > + elf->caller_xdest_base = hdr_ptr; > + elf->caller_xdest_size = size; > + hdr = ELF_REALPTR2PTRVAL(hdr_ptr); > dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend); > } > > @@ -169,9 +179,32 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom, > ehdr->e_shoff = elf_size(elf, elf->ehdr); > ehdr->e_shstrndx = SHN_UNDEF; > } > - if ( elf_init(&syms, hdr + sizeof(int), size - sizeof(int)) ) > + if ( elf->caller_xdest_size < sizeof(int) ) > + { > + DOMPRINTF("%s/%s: header size %"PRIx64" too small", > + __FUNCTION__, load ? "load" : "parse", > + (uint64_t)elf->caller_xdest_size); > + return -1; > + } > + if ( elf_init(&syms, elf->caller_xdest_base + sizeof(int), > + elf->caller_xdest_size - sizeof(int)) ) > return -1; > > + /* > + * The caller_xdest_{base,size} and dest_{base,size} need to > + * remain valid so long as each struct elf_image does. The > + * principle we adopt is that these values are set when the > + * memory is allocated or mapped, and cleared when (and if) > + * they are unmapped. > + * > + * Mappings of the guest are normally undone by xc_dom_unmap_all > + * (directly or via xc_dom_release). We do not explicitly clear > + * these because in fact that happens only at the end of > + * xc_dom_boot_image, at which time all of these ELF loading > + * functions have returned. No relevant struct elf_binary* > + * escapes this file. > + */ > + > xc_elf_set_logfile(dom->xch, &syms, 1); > > symtab = dom->bsd_symtab_start + sizeof(int); > @@ -310,8 +343,10 @@ static int xc_dom_load_elf_kernel(struct xc_dom_image *dom) > { > struct elf_binary *elf = dom->private_loader; > int rc; > + xen_pfn_t pages; > > - elf->dest = xc_dom_seg_to_ptr(dom, &dom->kernel_seg); > + elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg, &pages); > + elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom); Similarly here for pointer checking. > 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)) Similar query regarding elf_access_unsigned(elf, (h), 0, sizeof(*h)) > while ( STAR(h) ) > { > elf_memset_unchecked(name, 0, sizeof(name)); > diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c > index f7fe283..878552e 100644 > --- a/xen/common/libelf/libelf-loader.c > +++ b/xen/common/libelf/libelf-loader.c > @@ -24,23 +24,25 @@ > > /* ------------------------------------------------------------------------ */ > > -int elf_init(struct elf_binary *elf, const char *image, size_t size) > +int elf_init(struct elf_binary *elf, const char *image_input, size_t size) > { > ELF_HANDLE_DECL(elf_shdr) shdr; > uint64_t i, count, section, offset; > > - if ( !elf_is_elfbinary(image) ) > + if ( !elf_is_elfbinary(image_input) ) > { > elf_err(elf, "%s: not an ELF binary\n", __FUNCTION__); > return -1; > } > > elf_memset_unchecked(elf, 0, sizeof(*elf)); > - elf->image = image; > + elf->image_base = image_input; > elf->size = size; > - elf->ehdr = (elf_ehdr *)image; > - elf->class = elf->ehdr->e32.e_ident[EI_CLASS]; > - elf->data = elf->ehdr->e32.e_ident[EI_DATA]; > + elf->ehdr = ELF_MAKE_HANDLE(elf_ehdr, (elf_ptrval)image_input); > + elf->class = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_CLASS]); > + elf->data = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_DATA]); > + elf->caller_xdest_base = NULL; > + elf->caller_xdest_size = 0; > > /* Sanity check phdr. */ > offset = elf_uval(elf, elf->ehdr, e_phoff) + > @@ -300,7 +302,7 @@ int elf_load_binary(struct elf_binary *elf) > > ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr) > { > - return elf->dest + addr - elf->pstart; > + return ELF_REALPTR2PTRVAL(elf->dest_base) + addr - elf->pstart; Both callsites of this function have addr as a guest supplied parameter. It needs to be checked for overflowing. > } > > uint64_t elf_lookup_addr(struct elf_binary * elf, const char *symbol) > diff --git a/xen/common/libelf/libelf-private.h b/xen/common/libelf/libelf-private.h > index 0d4dcf6..0bd9e66 100644 > --- a/xen/common/libelf/libelf-private.h > +++ b/xen/common/libelf/libelf-private.h > @@ -86,6 +86,19 @@ do { strncpy((d),(s),sizeof((d))-1); \ > > #endif > > +#undef memcpy > +#undef memset > +#undef memmove > +#undef strcpy > + > +#define memcpy MISTAKE_unspecified_memcpy > +#define memset MISTAKE_unspecified_memset > +#define memmove MISTAKE_unspecified_memmove > +#define strcpy MISTAKE_unspecified_strcpy > + /* This prevents libelf from using these undecorated versions > + * of memcpy, memset, memmove and strcpy. Every call site > + * must either use elf_mem*_unchecked, or elf_mem*_safe. */ > + > #endif /* __LIBELF_PRIVATE_H_ */ > > /* > diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c > index fa7dedd..08ab027 100644 > --- a/xen/common/libelf/libelf-tools.c > +++ b/xen/common/libelf/libelf-tools.c > @@ -20,28 +20,100 @@ > > /* ------------------------------------------------------------------------ */ > > -uint64_t elf_access_unsigned(struct elf_binary * elf, const void *ptr, > - uint64_t offset, size_t size) > +void elf_mark_broken(struct elf_binary *elf, const char *msg) > { > + if ( elf->broken == NULL ) > + elf->broken = msg; > +} > + > +const char *elf_check_broken(const struct elf_binary *elf) > +{ > + return elf->broken; > +} > + > +static int elf_ptrval_in_range(elf_ptrval ptrval, uint64_t size, > + const void *region, uint64_t regionsize) > + /* > + * Returns true if the putative memory area [ptrval,ptrval+size> > + * is completely inside the region [region,region+regionsize>. > + * > + * ptrval and size are the untrusted inputs to be checked. > + * region and regionsize are trusted and must be correct and valid, > + * although it is OK for region to perhaps be maliciously NULL > + * (but not some other malicious value). > + */ > +{ > + elf_ptrval regionp = (elf_ptrval)region; > + > + if ( (region == NULL) || > + (ptrval < regionp) || /* start is before region */ > + (ptrval > regionp + regionsize) || /* start is after region */ > + (size > regionsize - (ptrval - regionp)) ) /* too big */ > + return 0; > + return 1; > +} > + > +int elf_access_ok(struct elf_binary * elf, > + uint64_t ptrval, size_t size) > +{ > + if ( elf_ptrval_in_range(ptrval, size, elf->image_base, elf->size) ) > + return 1; > + if ( elf_ptrval_in_range(ptrval, size, elf->dest_base, elf->dest_size) ) > + return 1; > + if ( elf_ptrval_in_range(ptrval, size, > + elf->caller_xdest_base, elf->caller_xdest_size) ) > + return 1; > + elf_mark_broken(elf, "out of range access"); > + return 0; > +} > + > +void elf_memcpy_safe(struct elf_binary *elf, elf_ptrval dst, > + elf_ptrval src, size_t size) > +{ > + if ( elf_access_ok(elf, dst, size) && > + elf_access_ok(elf, src, size) ) > + { > + /* use memmove because these checks do not prove that the > + * regions don't overlap and overlapping regions grant > + * permission for compiler malice */ > + elf_memmove_unchecked(ELF_UNSAFE_PTR(dst), ELF_UNSAFE_PTR(src), size); > + } > +} > + > +void elf_memset_safe(struct elf_binary *elf, elf_ptrval dst, int c, size_t size) > +{ > + if ( elf_access_ok(elf, dst, size) ) > + { > + elf_memset_unchecked(ELF_UNSAFE_PTR(dst), c, size); > + } > +} > + > +uint64_t elf_access_unsigned(struct elf_binary * elf, elf_ptrval base, > + uint64_t moreoffset, size_t size) > +{ > + elf_ptrval ptrval = base + moreoffset; > int need_swap = elf_swap(elf); > const uint8_t *u8; > const uint16_t *u16; > const uint32_t *u32; > const uint64_t *u64; > > + if ( !elf_access_ok(elf, ptrval, size) ) > + return 0; > + > switch ( size ) > { > case 1: > - u8 = ptr + offset; > + u8 = (const void*)ptrval; > return *u8; > case 2: > - u16 = ptr + offset; > + u16 = (const void*)ptrval; > return need_swap ? bswap_16(*u16) : *u16; > case 4: > - u32 = ptr + offset; > + u32 = (const void*)ptrval; > return need_swap ? bswap_32(*u32) : *u32; > case 8: > - u64 = ptr + offset; > + u64 = (const void*)ptrval; > return need_swap ? bswap_64(*u64) : *u64; > default: > return 0; > @@ -122,6 +194,28 @@ const char *elf_section_name(struct elf_binary *elf, > return elf_strval(elf, elf->sec_strtab + elf_uval(elf, shdr, sh_name)); > } > > +const char *elf_strval(struct elf_binary *elf, elf_ptrval start) > +{ > + uint64_t length; > + > + for ( length = 0; ; length++ ) { > + if ( !elf_access_ok(elf, start + length, 1) ) > + return NULL; > + if ( !elf_access_unsigned(elf, start, length, 1) ) > + /* ok */ > + return ELF_UNSAFE_PTR(start); > + } > +} > + > +const char *elf_strfmt(struct elf_binary *elf, elf_ptrval start) > +{ > + const char *str = elf_strval(elf, start); > + > + if ( str == NULL ) > + return "(invalid)"; > + return str; > +} > + > ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr) shdr) > { > return ELF_IMAGE_BASE(elf) + elf_uval(elf, shdr, sh_offset); > diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h > index 0c2c11e..783f125 100644 > --- a/xen/include/xen/libelf.h > +++ b/xen/include/xen/libelf.h > @@ -57,8 +57,9 @@ typedef void elf_log_callback(struct elf_binary*, void *caller_data, > * on this. > * This replaces variables which were char*,void* > * and their const versions, so we provide four > - * different declaration macros: > + * different obsolete declaration macros: > * ELF_PTRVAL_{,CONST}{VOID,CHAR} > + * New code can simply use the elf_ptrval typedef. > * HANDLE A pointer to a struct. There is one of these types > * for each pointer type - that is, for each "structname". > * In the arguments to the various HANDLE macros, structname > @@ -67,54 +68,66 @@ typedef void elf_log_callback(struct elf_binary*, void *caller_data, > * pointers. In the current code attempts to do so will > * compile, but in the next patch this will become a > * compile error. > - * We provide two declaration macros for const and > - * non-const pointers. > + * We also provide a second declaration macro for > + * pointers which were to const; this is obsolete. > */ > > -#define ELF_REALPTR2PTRVAL(realpointer) (realpointer) > +typedef uintptr_t elf_ptrval; > + > +#define ELF_REALPTR2PTRVAL(realpointer) ((elf_ptrval)(realpointer)) > /* Converts an actual C pointer into a PTRVAL */ > > -#define ELF_HANDLE_DECL_NONCONST(structname) structname * > -#define ELF_HANDLE_DECL(structname) const structname * > +#define ELF_HANDLE_DECL_NONCONST(structname) structname##_handle /*obsolete*/ > +#define ELF_HANDLE_DECL(structname) structname##_handle > /* Provides a type declaration for a HANDLE. */ > - /* May only be used to declare ONE variable at a time */ > > -#define ELF_PTRVAL_VOID void * > -#define ELF_PTRVAL_CHAR char * > -#define ELF_PTRVAL_CONST_VOID const void * > -#define ELF_PTRVAL_CONST_CHAR const char * > - /* Provides a type declaration for a PTRVAL. */ > - /* May only be used to declare ONE variable at a time */ > +#define ELF_PTRVAL_VOID elf_ptrval /*obsolete*/ > +#define ELF_PTRVAL_CHAR elf_ptrval /*obsolete*/ > +#define ELF_PTRVAL_CONST_VOID elf_ptrval /*obsolete*/ > +#define ELF_PTRVAL_CONST_CHAR elf_ptrval /*obsolete*/ > + > +#ifdef __XEN__ > +# define ELF_PRPTRVAL "lu" > + /* > + * PRIuPTR is misdefined in xen/include/xen/inttypes.h, on 32-bit, > + * to "u", when in fact uintptr_t is an unsigned long. > + */ > +#else > +# define ELF_PRPTRVAL PRIuPTR > +#endif > + /* printf format a la PRId... for a PTRVAL */ > > -#define ELF_DEFINE_HANDLE(structname) /* empty */ > +#define ELF_DEFINE_HANDLE(structname) \ > + typedef union { \ > + elf_ptrval ptrval; \ > + const structname *typeonly; /* for sizeof, offsetof, &c only */ \ > + } structname##_handle; > /* > * This must be invoked for each HANDLE type to define > * the actual C type used for that kind of HANDLE. > */ > > -#define ELF_PRPTRVAL "p" > - /* printf format a la PRId... for a PTRVAL */ > - > -#define ELF_MAKE_HANDLE(structname, ptrval) (ptrval) > +#define ELF_MAKE_HANDLE(structname, ptrval) ((structname##_handle){ ptrval }) > /* Converts a PTRVAL to a HANDLE */ > > -#define ELF_IMAGE_BASE(elf) ((elf)->image) > +#define ELF_IMAGE_BASE(elf) ((elf_ptrval)(elf)->image_base) > /* Returns the base of the image as a PTRVAL. */ > > -#define ELF_HANDLE_PTRVAL(handleval) ((void*)(handleval)) > +#define ELF_HANDLE_PTRVAL(handleval) ((handleval).ptrval) > /* Converts a HANDLE to a PTRVAL. */ > > -#define ELF_OBSOLETE_VOIDP_CAST (void*)(uintptr_t) > +#define ELF_OBSOLETE_VOIDP_CAST /*empty*/ > /* > - * In some places the existing code needs to > + * In some places the old code used to need to > * - cast away const (the existing code uses const a fair > * bit but actually sometimes wants to write to its input) > * from a PTRVAL. > * - convert an integer representing a pointer to a PTRVAL > - * This macro provides a suitable cast. > + * Nowadays all of these re uintptr_ts so there is no const problem > + * and no need for any casting. > */ > > -#define ELF_UNSAFE_PTR(ptrval) ((void*)(ptrval)) > +#define ELF_UNSAFE_PTR(ptrval) ((void*)(elf_ptrval)(ptrval)) > /* > * Turns a PTRVAL into an actual C pointer. Before this is done > * the caller must have ensured that the PTRVAL does in fact point > @@ -122,23 +135,25 @@ typedef void elf_log_callback(struct elf_binary*, void *caller_data, > */ > > /* PTRVALs can be INVALID (ie, NULL). */ > -#define ELF_INVALID_PTRVAL (NULL) /* returns NULL PTRVAL */ > +#define ELF_INVALID_PTRVAL ((elf_ptrval)0) /* returns NULL PTRVAL */ > #define ELF_INVALID_HANDLE(structname) /* returns NULL handle */ \ > ELF_MAKE_HANDLE(structname, ELF_INVALID_PTRVAL) > -#define ELF_PTRVAL_VALID(ptrval) (ptrval) /* } */ > -#define ELF_HANDLE_VALID(handleval) (handleval) /* } predicates */ > -#define ELF_PTRVAL_INVALID(ptrval) ((ptrval) == NULL) /* } */ > +#define ELF_PTRVAL_VALID(ptrval) (!!(ptrval)) /* } */ > +#define ELF_HANDLE_VALID(handleval) (!!(handleval).ptrval) /* } predicates */ > +#define ELF_PTRVAL_INVALID(ptrval) (!ELF_PTRVAL_VALID((ptrval))) /* } */ > + > +#define ELF_MAX_PTRVAL (~(elf_ptrval)0) > + /* PTRVAL value guaranteed to compare > to any valid PTRVAL */ > > /* For internal use by other macros here */ > #define ELF__HANDLE_FIELD_TYPE(handleval, elm) \ > - typeof((handleval)->elm) > + typeof((handleval).typeonly->elm) > #define ELF__HANDLE_FIELD_OFFSET(handleval, elm) \ > - offsetof(typeof(*(handleval)),elm) > + offsetof(typeof(*(handleval).typeonly),elm) > > > /* ------------------------------------------------------------------------ */ > > - Spurious whitespace change. ~Andrew > typedef union { > Elf32_Ehdr e32; > Elf64_Ehdr e64; > @@ -182,7 +197,7 @@ ELF_DEFINE_HANDLE(elf_note) > > struct elf_binary { > /* elf binary */ > - const char *image; > + const void *image_base; > size_t size; > char class; > char data; > @@ -190,10 +205,16 @@ struct elf_binary { > ELF_HANDLE_DECL(elf_ehdr) ehdr; > ELF_PTRVAL_CONST_CHAR sec_strtab; > ELF_HANDLE_DECL(elf_shdr) sym_tab; > - ELF_PTRVAL_CONST_CHAR sym_strtab; > + uint64_t sym_strtab; > > /* loaded to */ > - char *dest; > + /* > + * dest_base and dest_size are trusted and must be correct; > + * whenever dest_size is not 0, both of these must be valid > + * so long as the struct elf_binary is in use. > + */ > + char *dest_base; > + size_t dest_size; > uint64_t pstart; > uint64_t pend; > uint64_t reloc_offset; > @@ -201,12 +222,22 @@ struct elf_binary { > uint64_t bsd_symtab_pstart; > uint64_t bsd_symtab_pend; > > + /* > + * caller's other acceptable destination > + * > + * Again, these are trusted and must be valid (or 0) so long > + * as the struct elf_binary is in use. > + */ > + void *caller_xdest_base; > + uint64_t caller_xdest_size; > + > #ifndef __XEN__ > /* misc */ > elf_log_callback *log_callback; > void *log_caller_data; > #endif > int verbose; > + const char *broken; > }; > > /* ------------------------------------------------------------------------ */ > @@ -224,22 +255,27 @@ struct elf_binary { > #define elf_lsb(elf) (ELFDATA2LSB == (elf)->data) > #define elf_swap(elf) (NATIVE_ELFDATA != (elf)->data) > > -#define elf_uval(elf, str, elem) \ > - ((ELFCLASS64 == (elf)->class) \ > - ? elf_access_unsigned((elf), (str), \ > - offsetof(typeof(*(str)),e64.elem), \ > - sizeof((str)->e64.elem)) \ > - : elf_access_unsigned((elf), (str), \ > - offsetof(typeof(*(str)),e32.elem), \ > - sizeof((str)->e32.elem))) > +#define elf_uval_3264(elf, handle, elem) \ > + elf_access_unsigned((elf), (handle).ptrval, \ > + offsetof(typeof(*(handle).typeonly),elem), \ > + sizeof((handle).typeonly->elem)) > + > +#define elf_uval(elf, handle, elem) \ > + ((ELFCLASS64 == (elf)->class) \ > + ? elf_uval_3264(elf, handle, e64.elem) \ > + : elf_uval_3264(elf, handle, e32.elem)) > /* > * Reads an unsigned field in a header structure in the ELF. > * str is a HANDLE, and elem is the field name in it. > */ > > -#define elf_size(elf, str) \ > + > +#define elf_size(elf, handle_or_handletype) ({ \ > + typeof(handle_or_handletype) elf_size__dummy; \ > ((ELFCLASS64 == (elf)->class) \ > - ? sizeof((str)->e64) : sizeof((str)->e32)) > + ? sizeof(elf_size__dummy.typeonly->e64) \ > + : sizeof(elf_size__dummy.typeonly->e32)); \ > +}) > /* > * Returns the size of the substructure for the appropriate 32/64-bitness. > * str should be a HANDLE. > @@ -251,23 +287,37 @@ uint64_t elf_access_unsigned(struct elf_binary *elf, ELF_PTRVAL_CONST_VOID ptr, > > uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr); > > +const char *elf_strval(struct elf_binary *elf, elf_ptrval start); > + /* may return NULL if the string is out of range etc. */ > > -#define elf_strval(elf,x) ((const char*)(x)) /* may return NULL in the future */ > -#define elf_strfmt(elf,x) ((const char*)(x)) /* will return (invalid) instead */ > +const char *elf_strfmt(struct elf_binary *elf, elf_ptrval start); > + /* like elf_strval but returns "(invalid)" instead of NULL */ > > -#define elf_memcpy_safe(elf, dst, src, sz) memcpy((dst),(src),(sz)) > -#define elf_memset_safe(elf, dst, c, sz) memset((dst),(c),(sz)) > +void elf_memcpy_safe(struct elf_binary*, elf_ptrval dst, elf_ptrval src, size_t); > +void elf_memset_safe(struct elf_binary*, elf_ptrval dst, int c, size_t); > /* > - * Versions of memcpy and memset which will (in the next patch) > - * arrange never to write outside permitted areas. > + * Versions of memcpy and memset which arrange never to write > + * outside permitted areas. > */ > > -#define elf_store_val(elf, type, ptr, val) (*(type*)(ptr) = (val)) > +int elf_access_ok(struct elf_binary * elf, > + uint64_t ptrval, size_t size); > + > +#define elf_store_val(elf, type, ptr, val) \ > + ({ \ > + typeof(type) elf_store__val = (val); \ > + elf_ptrval elf_store__targ = ptr; \ > + if (elf_access_ok((elf), elf_store__targ, \ > + sizeof(elf_store__val))) { \ > + elf_memcpy_unchecked((void*)elf_store__targ, &elf_store__val, \ > + sizeof(elf_store__val)); \ > + } \ > + }) \ > /* Stores a value at a particular PTRVAL. */ > > -#define elf_store_field(elf, hdr, elm, val) \ > - (elf_store_val((elf), ELF__HANDLE_FIELD_TYPE(hdr, elm), \ > - &((hdr)->elm), \ > +#define elf_store_field(elf, hdr, elm, val) \ > + (elf_store_val((elf), ELF__HANDLE_FIELD_TYPE(hdr, elm), \ > + ELF_HANDLE_PTRVAL(hdr) + ELF__HANDLE_FIELD_OFFSET(hdr, elm), \ > (val))) > /* Stores a 32/64-bit field. hdr is a HANDLE and elm is the field name. */ > > @@ -306,6 +356,10 @@ int elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr) > /* xc_libelf_loader.c */ > > int elf_init(struct elf_binary *elf, const char *image, size_t size); > + /* > + * image and size must be correct. They will be recorded in > + * *elf, and must remain valid while the elf is in use. > + */ > #ifdef __XEN__ > void elf_set_verbose(struct elf_binary *elf); > #else > @@ -321,6 +375,9 @@ uint64_t elf_lookup_addr(struct elf_binary *elf, const char *symbol); > > void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart); /* private */ > > +void elf_mark_broken(struct elf_binary *elf, const char *msg); > +const char *elf_check_broken(const struct elf_binary *elf); /* NULL means OK */ > + > /* ------------------------------------------------------------------------ */ > /* xc_libelf_relocate.c */ > > @@ -395,16 +452,38 @@ int elf_xen_parse_guest_info(struct elf_binary *elf, > int elf_xen_parse(struct elf_binary *elf, > struct elf_dom_parms *parms); > > -#define elf_memcpy_unchecked memcpy > -#define elf_memset_unchecked memset > +static inline void *elf_memcpy_unchecked(void *dest, const void *src, size_t n) > + { return memcpy(dest, src, n); } > +static inline void *elf_memmove_unchecked(void *dest, const void *src, size_t n) > + { return memmove(dest, src, n); } > +static inline void *elf_memset_unchecked(void *s, int c, size_t n) > + { return memset(s, c, n); } > /* > - * Unsafe versions of memcpy and memset which take actual C > - * pointers. These are just like real memcpy and memset. > + * Unsafe versions of memcpy, memmove memset which take actual C > + * pointers. These are just like the real functions. > + * We provide these so that in libelf-private.h we can #define > + * memcpy, memset and memmove to undefined MISTAKE things. > */ > > > -#define ELF_ADVANCE_DEST(elf, amount) elf->dest += (amount) > - /* Advances past amount bytes of the current destination area. */ > +/* Advances past amount bytes of the current destination area. */ > +static inline void ELF_ADVANCE_DEST(struct elf_binary *elf, uint64_t amount) > +{ > + if ( elf->dest_base == NULL ) > + { > + elf_mark_broken(elf, "advancing in null image"); > + } > + else if ( elf->dest_size >= amount ) > + { > + elf->dest_base += amount; > + elf->dest_size -= amount; > + } > + else > + { > + elf->dest_size = 0; > + elf_mark_broken(elf, "advancing past end (image very short?)"); > + } > +} > > > #endif /* __XEN_LIBELF_H__ */ > -- > 1.7.2.5 >