Re: [PATCH 15/22] libelf: use only unsigned integers

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 11/06/13 19:20, Ian Jackson wrote:
> Signed integers have undesirable undefined behaviours on overflow.
> Malicious compilers can turn apparently-correct code into code with
> security vulnerabilities etc.
>
> So use only unsigned integers.  Exceptions are booleans (which we have
> already changed) and error codes.
>
> We _do_ change all the chars which aren't fixed constants from our own
> text segment, but not the char*s.  This is because it is safe to
> access an arbitrary byte through a char*, but not necessarily safe to
> convert an arbitrary value to a char.
>
> As a consequence we need to compile libelf with -Wno-pointer-sign.
>
> It is OK to change all the signed integers to unsigned because all the
> inequalities in libelf are in contexts where we don't "expect"
> negative numbers.
>
> In libelf-dominfo.c:elf_xen_parse we rename a variable "rc" to
> "more_notes" as it actually contains a note count derived from the
> input image.  The "error" return value from elf_xen_parse_notes is
> changed from -1 to ~0U.
>
> grepping shows only one occurrence of "PRId" or "%d" or "%ld" in
> libelf and xc_dom_elfloader.c (a "%d" which becomes "%u").
>
> This is part of the fix to a security issue, XSA-55.
>
> For those concerned about unintentional functional changes, the
> following rune produces a version of the patch which is much smaller
> and eliminates only non-functional changes:
>
>  GIT_EXTERNAL_DIFF=.../unsigned-differ git-diff <before>..<after>
>
> where <before> and <after> are git refs for the code before and after
> this patch, and unsigned-differ is this shell script:
>
>     #!/bin/bash
>     set -e
>
>     seddery () {
>             perl -pe 's/\b(?:elf_errorstatus|elf_negerrnoval)\b/int/g'
>     }
>
>     path="$1"
>     in="$2"
>     out="$5"
>
>     set +e
>     diff -pu --label "$path~" <(seddery <"$in") --label "$path" <(seddery <"$out")
>     rc=$?
>     set -e
>     if [ $rc = 1 ]; then rc=0; fi
>     exit $rc
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> v5: Introduce ELF_NOTE_INVALID, instead of using a literal ~0U.
>
> v4: Fix regression in elf_round_up; use uint64_t here.
>
> v3: Changes to booleans split off into separate patch.
>
> v2: BUGFIX: Eliminate conversion to int of return from elf_xen_parse_notes.
>     BUGFIX: Fix the one printf format thing which needs changing.
>     Remove irrelevant change to constify note_desc.name in libelf-dominfo.c.
>     In xc_dom_load_elf_symtab change one sizeof(int) to sizeof(unsigned).
>     Do not change type of 2nd argument to memset.
>     Provide seddery for easier review.
>     Style fix.
> ---
>  tools/libxc/Makefile               |    9 +++++-
>  tools/libxc/xc_dom.h               |    7 +++--
>  tools/libxc/xc_dom_elfloader.c     |   42 ++++++++++++++++-------------
>  tools/xcutils/readnotes.c          |   15 +++++-----
>  xen/common/libelf/Makefile         |    2 +
>  xen/common/libelf/libelf-dominfo.c |   52 ++++++++++++++++++-----------------
>  xen/common/libelf/libelf-loader.c  |   20 +++++++-------
>  xen/common/libelf/libelf-tools.c   |   24 ++++++++--------
>  xen/include/xen/libelf.h           |   21 ++++++++------
>  9 files changed, 105 insertions(+), 87 deletions(-)
>
> diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
> index 4a31282..512a994 100644
> --- a/tools/libxc/Makefile
> +++ b/tools/libxc/Makefile
> @@ -51,8 +51,13 @@ endif
>  vpath %.c ../../xen/common/libelf
>  CFLAGS += -I../../xen/common/libelf
>
> -GUEST_SRCS-y += libelf-tools.c libelf-loader.c
> -GUEST_SRCS-y += libelf-dominfo.c
> +ELF_SRCS-y += libelf-tools.c libelf-loader.c
> +ELF_SRCS-y += libelf-dominfo.c
> +
> +GUEST_SRCS-y += $(ELF_SRCS-y)
> +
> +$(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
> +$(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
>
>  # new domain builder
>  GUEST_SRCS-y                 += xc_dom_core.c xc_dom_boot.c
> diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
> index ad6fdd4..5968e7b 100644
> --- a/tools/libxc/xc_dom.h
> +++ b/tools/libxc/xc_dom.h
> @@ -155,9 +155,10 @@ struct xc_dom_image {
>
>  struct xc_dom_loader {
>      char *name;
> -    int (*probe) (struct xc_dom_image * dom);
> -    int (*parser) (struct xc_dom_image * dom);
> -    int (*loader) (struct xc_dom_image * dom);
> +    /* Sadly the error returns from these functions are not consistent: */
> +    elf_negerrnoval (*probe) (struct xc_dom_image * dom);
> +    elf_negerrnoval (*parser) (struct xc_dom_image * dom);
> +    elf_errorstatus (*loader) (struct xc_dom_image * dom);
>
>      struct xc_dom_loader *next;
>  };
> diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
> index 8f9c2fb..eb2e3d2 100644
> --- a/tools/libxc/xc_dom_elfloader.c
> +++ b/tools/libxc/xc_dom_elfloader.c
> @@ -82,7 +82,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
>  /* ------------------------------------------------------------------------ */
>  /* parse elf binary                                                         */
>
> -static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
> +static elf_negerrnoval check_elf_kernel(struct xc_dom_image *dom, bool verbose)
>  {
>      if ( dom->kernel_blob == NULL )
>      {
> @@ -104,12 +104,12 @@ static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
>      return 0;
>  }
>
> -static int xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
> +static elf_negerrnoval xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
>  {
>      return check_elf_kernel(dom, 0);
>  }
>
> -static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
> +static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>                                    struct elf_binary *elf, bool load)
>  {
>      struct elf_binary syms;
> @@ -117,7 +117,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>      xen_vaddr_t symtab, maxaddr;
>      ELF_PTRVAL_CHAR hdr;
>      size_t size;
> -    int h, count, type, i, tables = 0;
> +    unsigned h, count, type, i, tables = 0;
>
>      if ( elf_swap(elf) )
>      {
> @@ -138,13 +138,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>          elf->caller_xdest_base = hdr_ptr;
>          elf->caller_xdest_size = allow_size;
>          hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
> -        elf_store_val(elf, int, hdr, size - sizeof(int));
> +        elf_store_val(elf, unsigned, hdr, size - sizeof(unsigned));
>      }
>      else
>      {
>          char *hdr_ptr;
>
> -        size = sizeof(int) + elf_size(elf, elf->ehdr) +
> +        size = sizeof(unsigned) + elf_size(elf, elf->ehdr) +
>              elf_shdr_count(elf) * elf_size(elf, shdr);
>          hdr_ptr = xc_dom_malloc(dom, size);
>          if ( hdr_ptr == NULL )
> @@ -155,15 +155,15 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>          dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
>      }
>
> -    elf_memcpy_safe(elf, hdr + sizeof(int),
> +    elf_memcpy_safe(elf, hdr + sizeof(unsigned),
>             ELF_IMAGE_BASE(elf),
>             elf_size(elf, elf->ehdr));
> -    elf_memcpy_safe(elf, hdr + sizeof(int) + elf_size(elf, elf->ehdr),
> +    elf_memcpy_safe(elf, hdr + sizeof(unsigned) + elf_size(elf, elf->ehdr),
>             ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
>             elf_shdr_count(elf) * elf_size(elf, shdr));
>      if ( elf_64bit(elf) )
>      {
> -        Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(int));
> +        Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(unsigned));
>          ehdr->e_phoff = 0;
>          ehdr->e_phentsize = 0;
>          ehdr->e_phnum = 0;
> @@ -172,22 +172,22 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>      }
>      else
>      {
> -        Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(int));
> +        Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(unsigned));
>          ehdr->e_phoff = 0;
>          ehdr->e_phentsize = 0;
>          ehdr->e_phnum = 0;
>          ehdr->e_shoff = elf_size(elf, elf->ehdr);
>          ehdr->e_shstrndx = SHN_UNDEF;
>      }
> -    if ( elf->caller_xdest_size < sizeof(int) )
> +    if ( elf->caller_xdest_size < sizeof(unsigned) )
>      {
>          DOMPRINTF("%s/%s: header size %"PRIx64" too small",
>                    __FUNCTION__, load ? "load" : "parse",
>                    (uint64_t)elf->caller_xdest_size);
>          return -1;
>      }
> -    if ( elf_init(&syms, elf->caller_xdest_base + sizeof(int),
> -                  elf->caller_xdest_size - sizeof(int)) )
> +    if ( elf_init(&syms, elf->caller_xdest_base + sizeof(unsigned),
> +                  elf->caller_xdest_size - sizeof(unsigned)) )
>          return -1;
>
>      /*
> @@ -207,7 +207,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>
>      xc_elf_set_logfile(dom->xch, &syms, 1);
>
> -    symtab = dom->bsd_symtab_start + sizeof(int);
> +    symtab = dom->bsd_symtab_start + sizeof(unsigned);
>      maxaddr = elf_round_up(&syms, symtab + elf_size(&syms, syms.ehdr) +
>                             elf_shdr_count(&syms) * elf_size(&syms, shdr));
>
> @@ -253,7 +253,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>              size = elf_uval(&syms, shdr, sh_size);
>              maxaddr = elf_round_up(&syms, maxaddr + size);
>              tables++;
> -            DOMPRINTF("%s: h=%d %s, size=0x%zx, maxaddr=0x%" PRIx64 "",
> +            DOMPRINTF("%s: h=%u %s, size=0x%zx, maxaddr=0x%" PRIx64 "",
>                        __FUNCTION__, h,
>                        type == SHT_SYMTAB ? "symtab" : "strtab",
>                        size, maxaddr);
> @@ -292,10 +292,14 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
>      return 0;
>  }
>
> -static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
> +static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
> +    /*
> +     * This function sometimes returns -1 for error and sometimes
> +     * an errno value.  WTF?
> +     */
>  {
>      struct elf_binary *elf;
> -    int rc;
> +    elf_errorstatus rc;
>
>      rc = check_elf_kernel(dom, 1);
>      if ( rc != 0 )
> @@ -356,10 +360,10 @@ out:
>      return rc;
>  }
>
> -static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
> +static elf_errorstatus xc_dom_load_elf_kernel(struct xc_dom_image *dom)
>  {
>      struct elf_binary *elf = dom->private_loader;
> -    int rc;
> +    elf_errorstatus rc;
>      xen_pfn_t pages;
>
>      elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg, &pages);
> diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
> index d1f7a30..2ca7732 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) )
> @@ -161,7 +161,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;
>                 }
>         }
> @@ -171,12 +171,13 @@ static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start,
>  int main(int argc, char **argv)
>  {
>         const char *f;
> -       int fd,h,size,usize,count;
> +       int fd;
> +       unsigned h,size,usize,count;
>         void *image,*tmp;
>         struct stat st;
>         struct elf_binary elf;
>         ELF_HANDLE_DECL(elf_shdr) shdr;
> -       int notes_found = 0;
> +       unsigned notes_found = 0;
>
>         struct setup_header *hdr;
>         uint64_t payload_offset, payload_length;
> diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
> index 18dc8e2..5bf8f76 100644
> --- a/xen/common/libelf/Makefile
> +++ b/xen/common/libelf/Makefile
> @@ -2,6 +2,8 @@ obj-bin-y := libelf.o
>
>  SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
>
> +CFLAGS += -Wno-pointer-sign
> +
>  libelf.o: libelf-temp.o Makefile
>         $(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
>
> diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
> index c4ced67..0b07002 100644
> --- a/xen/common/libelf/libelf-dominfo.c
> +++ b/xen/common/libelf/libelf-dominfo.c
> @@ -29,15 +29,15 @@ static const char *const elf_xen_feature_names[] = {
>      [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
>      [XENFEAT_dom0] = "dom0"
>  };
> -static const int elf_xen_features =
> +static const unsigned elf_xen_features =
>  sizeof(elf_xen_feature_names) / sizeof(elf_xen_feature_names[0]);
>
> -int elf_xen_parse_features(const char *features,
> +elf_errorstatus elf_xen_parse_features(const char *features,
>                             uint32_t *supported,
>                             uint32_t *required)
>  {
> -    char feature[64];
> -    int pos, len, i;
> +    unsigned char feature[64];
> +    unsigned pos, len, i;
>
>      if ( features == NULL )
>          return 0;
> @@ -94,7 +94,7 @@ int elf_xen_parse_features(const char *features,
>  /* ------------------------------------------------------------------------ */
>  /* xen elf notes                                                            */
>
> -int elf_xen_parse_note(struct elf_binary *elf,
> +elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
>                         struct elf_dom_parms *parms,
>                         ELF_HANDLE_DECL(elf_note) note)
>  {
> @@ -125,7 +125,7 @@ int elf_xen_parse_note(struct elf_binary *elf,
>      const char *str = NULL;
>      uint64_t val = 0;
>      unsigned int i;
> -    int type = elf_uval(elf, note, type);
> +    unsigned type = elf_uval(elf, note, type);
>
>      if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
>           (note_desc[type].name == NULL) )
> @@ -216,12 +216,14 @@ int elf_xen_parse_note(struct elf_binary *elf,
>      return 0;
>  }
>
> -static int elf_xen_parse_notes(struct elf_binary *elf,
> +#define ELF_NOTE_INVALID (~0U)
> +
> +static unsigned elf_xen_parse_notes(struct elf_binary *elf,
>                                 struct elf_dom_parms *parms,
>                                 ELF_PTRVAL_CONST_VOID start,
>                                 ELF_PTRVAL_CONST_VOID end)
>  {
> -    int xen_elfnotes = 0;
> +    unsigned xen_elfnotes = 0;
>      ELF_HANDLE_DECL(elf_note) note;
>      const char *note_name;
>
> @@ -237,7 +239,7 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
>          if ( strcmp(note_name, "Xen") )
>              continue;
>          if ( elf_xen_parse_note(elf, parms, note) )
> -            return -1;
> +            return ELF_NOTE_INVALID;
>          xen_elfnotes++;
>      }
>      return xen_elfnotes;
> @@ -246,12 +248,12 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
>  /* ------------------------------------------------------------------------ */
>  /* __xen_guest section                                                      */
>
> -int elf_xen_parse_guest_info(struct elf_binary *elf,
> +elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
>                               struct elf_dom_parms *parms)
>  {
>      ELF_PTRVAL_CONST_CHAR h;
> -    char name[32], value[128];
> -    int len;
> +    unsigned char name[32], value[128];
> +    unsigned len;
>
>      h = parms->guest_info;
>  #define STAR(h) (elf_access_unsigned(elf, (h), 0, 1))
> @@ -334,13 +336,13 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
>  /* ------------------------------------------------------------------------ */
>  /* sanity checks                                                            */
>
> -static int elf_xen_note_check(struct elf_binary *elf,
> +static elf_errorstatus elf_xen_note_check(struct elf_binary *elf,
>                                struct elf_dom_parms *parms)
>  {
>      if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) &&
>           (ELF_PTRVAL_INVALID(parms->guest_info)) )
>      {
> -        int machine = elf_uval(elf, elf->ehdr, e_machine);
> +        unsigned machine = elf_uval(elf, elf->ehdr, e_machine);
>          if ( (machine == EM_386) || (machine == EM_X86_64) )
>          {
>              elf_err(elf, "%s: ERROR: Not a Xen-ELF image: "
> @@ -378,7 +380,7 @@ static int elf_xen_note_check(struct elf_binary *elf,
>      return 0;
>  }
>
> -static int elf_xen_addr_calc_check(struct elf_binary *elf,
> +static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf,
>                                     struct elf_dom_parms *parms)
>  {
>      if ( (parms->elf_paddr_offset != UNSET_ADDR) &&
> @@ -464,13 +466,13 @@ static int elf_xen_addr_calc_check(struct elf_binary *elf,
>  /* ------------------------------------------------------------------------ */
>  /* glue it all together ...                                                 */
>
> -int elf_xen_parse(struct elf_binary *elf,
> +elf_errorstatus elf_xen_parse(struct elf_binary *elf,
>                    struct elf_dom_parms *parms)
>  {
>      ELF_HANDLE_DECL(elf_shdr) shdr;
>      ELF_HANDLE_DECL(elf_phdr) phdr;
> -    int xen_elfnotes = 0;
> -    int i, count, rc;
> +    unsigned xen_elfnotes = 0;
> +    unsigned i, count, more_notes;
>
>      elf_memset_unchecked(parms, 0, sizeof(*parms));
>      parms->virt_base = UNSET_ADDR;
> @@ -495,13 +497,13 @@ int elf_xen_parse(struct elf_binary *elf,
>          if (elf_uval(elf, phdr, p_offset) == 0)
>               continue;
>
> -        rc = elf_xen_parse_notes(elf, parms,
> +        more_notes = elf_xen_parse_notes(elf, parms,
>                                   elf_segment_start(elf, phdr),
>                                   elf_segment_end(elf, phdr));
> -        if ( rc == -1 )
> +        if ( more_notes == ELF_NOTE_INVALID )
>              return -1;
>
> -        xen_elfnotes += rc;
> +        xen_elfnotes += more_notes;
>      }
>
>      /*
> @@ -518,17 +520,17 @@ int elf_xen_parse(struct elf_binary *elf,
>              if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
>                  continue;
>
> -            rc = elf_xen_parse_notes(elf, parms,
> +            more_notes = elf_xen_parse_notes(elf, parms,
>                                       elf_section_start(elf, shdr),
>                                       elf_section_end(elf, shdr));
>
> -            if ( rc == -1 )
> +            if ( more_notes == ELF_NOTE_INVALID )
>                  return -1;
>
> -            if ( xen_elfnotes == 0 && rc > 0 )
> +            if ( xen_elfnotes == 0 && more_notes > 0 )
>                  elf_msg(elf, "%s: using notes from SHT_NOTE section\n", __FUNCTION__);
>
> -            xen_elfnotes += rc;
> +            xen_elfnotes += more_notes;
>          }
>
>      }
> diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
> index 798f88b..937c99b 100644
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -24,7 +24,7 @@
>
>  /* ------------------------------------------------------------------------ */
>
> -int elf_init(struct elf_binary *elf, const char *image_input, size_t size)
> +elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t size)
>  {
>      ELF_HANDLE_DECL(elf_shdr) shdr;
>      uint64_t i, count, section, offset;
> @@ -114,7 +114,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback *log_callback,
>      elf->verbose = verbose;
>  }
>
> -static int elf_load_image(struct elf_binary *elf,
> +static elf_errorstatus elf_load_image(struct elf_binary *elf,
>                            ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src,
>                            uint64_t filesz, uint64_t memsz)
>  {
> @@ -129,9 +129,9 @@ void elf_set_verbose(struct elf_binary *elf)
>      elf->verbose = 1;
>  }
>
> -static int elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
> +static elf_errorstatus elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
>  {
> -    int rc;
> +    elf_errorstatus rc;
>      if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
>          return -1;
>      /* We trust the dom0 kernel image completely, so we don't care
> @@ -151,7 +151,7 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart)
>  {
>      uint64_t sz;
>      ELF_HANDLE_DECL(elf_shdr) shdr;
> -    int i, type;
> +    unsigned i, type;
>
>      if ( !ELF_HANDLE_VALID(elf->sym_tab) )
>          return;
> @@ -187,7 +187,7 @@ static void elf_load_bsdsyms(struct elf_binary *elf)
>      ELF_PTRVAL_VOID symbase;
>      ELF_PTRVAL_VOID symtab_addr;
>      ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr;
> -    int i, type;
> +    unsigned i, type;
>
>      if ( !elf->bsd_symtab_pstart )
>          return;
> @@ -220,7 +220,7 @@ do {                                            \
>      elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(shdr),
>                      ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
>                      sz);
> -    maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
> +    maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva + sz);
>
>      for ( i = 0; i < elf_shdr_count(elf); i++ )
>      {
> @@ -233,10 +233,10 @@ do {                                            \
>               elf_memcpy_safe(elf, maxva, elf_section_start(elf, shdr), sz);
>               /* Mangled to be based on ELF header location. */
>               elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
> -             maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
> +             maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva + sz);
>          }
>          shdr = ELF_MAKE_HANDLE(elf_shdr, ELF_HANDLE_PTRVAL(shdr) +
> -                            (long)elf_uval(elf, elf->ehdr, e_shentsize));
> +                            (unsigned long)elf_uval(elf, elf->ehdr, e_shentsize));
>      }
>
>      /* Write down the actual sym size. */
> @@ -273,7 +273,7 @@ void elf_parse_binary(struct elf_binary *elf)
>              __FUNCTION__, elf->pstart, elf->pend);
>  }
>
> -int elf_load_binary(struct elf_binary *elf)
> +elf_errorstatus elf_load_binary(struct elf_binary *elf)
>  {
>      ELF_HANDLE_DECL(elf_phdr) phdr;
>      uint64_t i, count, paddr, offset, filesz, memsz;
> diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
> index 0b7b2b6..6543f33 100644
> --- a/xen/common/libelf/libelf-tools.c
> +++ b/xen/common/libelf/libelf-tools.c
> @@ -122,19 +122,19 @@ uint64_t elf_access_unsigned(struct elf_binary * elf, elf_ptrval base,
>
>  uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr)
>  {
> -    int elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
> +    uint64_t elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
>
>      return (addr + elf_round) & ~elf_round;
>  }
>
>  /* ------------------------------------------------------------------------ */
>
> -int elf_shdr_count(struct elf_binary *elf)
> +unsigned elf_shdr_count(struct elf_binary *elf)
>  {
>      return elf_uval(elf, elf->ehdr, e_shnum);
>  }
>
> -int elf_phdr_count(struct elf_binary *elf)
> +unsigned elf_phdr_count(struct elf_binary *elf)
>  {
>      return elf_uval(elf, elf->ehdr, e_phnum);
>  }
> @@ -144,7 +144,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *n
>      uint64_t count = elf_shdr_count(elf);
>      ELF_HANDLE_DECL(elf_shdr) shdr;
>      const char *sname;
> -    int i;
> +    unsigned i;
>
>      for ( i = 0; i < count; i++ )
>      {
> @@ -156,7 +156,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *n
>      return ELF_INVALID_HANDLE(elf_shdr);
>  }
>
> -ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
> +ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned index)
>  {
>      uint64_t count = elf_shdr_count(elf);
>      ELF_PTRVAL_CONST_VOID ptr;
> @@ -170,7 +170,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
>      return ELF_MAKE_HANDLE(elf_shdr, ptr);
>  }
>
> -ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index)
> +ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned index)
>  {
>      uint64_t count = elf_uval(elf, elf->ehdr, e_phnum);
>      ELF_PTRVAL_CONST_VOID ptr;
> @@ -264,7 +264,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char *sym
>      return ELF_INVALID_HANDLE(elf_sym);
>  }
>
> -ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index)
> +ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned index)
>  {
>      ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
>      ELF_HANDLE_DECL(elf_sym) sym;
> @@ -280,7 +280,7 @@ const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note
>
>  ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
>  {
> -    int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
> +    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
>
>      return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz;
>  }
> @@ -288,7 +288,7 @@ ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_
>  uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
>  {
>      ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
> -    int descsz = elf_uval(elf, note, descsz);
> +    unsigned descsz = elf_uval(elf, note, descsz);
>
>      switch (descsz)
>      {
> @@ -306,7 +306,7 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note
>                                  unsigned int unitsz, unsigned int idx)
>  {
>      ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
> -    int descsz = elf_uval(elf, note, descsz);
> +    unsigned descsz = elf_uval(elf, note, descsz);
>
>      if ( descsz % unitsz || idx >= descsz / unitsz )
>          return 0;
> @@ -324,8 +324,8 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note
>
>  ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
>  {
> -    int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
> -    int descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
> +    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
> +    unsigned descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
>
>      return ELF_MAKE_HANDLE(elf_note, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz + descsz);
>  }
> diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
> index 32b3ce2..87e6f40 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__
> @@ -328,12 +331,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);
> @@ -343,7 +346,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);
> @@ -360,7 +363,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.
> @@ -373,7 +376,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);
> @@ -386,7 +389,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                                                      */
> @@ -420,7 +423,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
>