xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: andrew.cooper3@citrix.com, mattjd@gmail.com,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	security@xen.org
Subject: [PATCH 15/22] libelf: use only unsigned integers
Date: Fri, 7 Jun 2013 19:27:15 +0100	[thread overview]
Message-ID: <1370629642-6990-16-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1370629642-6990-1-git-send-email-ian.jackson@eu.citrix.com>

Signed integers have undesirable undefined behaviours on overflow.
Malicious compilers can turn apparently-correct code into code with
security vulnerabilities etc.

So use only unsigned integers.  Exceptions are booleans (which we have
already changed) and error codes.

We _do_ change all the chars which aren't fixed constants from our own
text segment, but not the char*s.  This is because it is safe to
access an arbitrary byte through a char*, but not necessarily safe to
convert an arbitrary value to a char.

As a consequence we need to compile libelf with -Wno-pointer-sign.

It is OK to change all the signed integers to unsigned because all the
inequalities in libelf are in contexts where we don't "expect"
negative numbers.

In libelf-dominfo.c:elf_xen_parse we rename a variable "rc" to
"more_notes" as it actually contains a note count derived from the
input image.  The "error" return value from elf_xen_parse_notes is
changed from -1 to ~0U.

grepping shows only one occurrence of "PRId" or "%d" or "%ld" in
libelf and xc_dom_elfloader.c (a "%d" which becomes "%u").

This is part of the fix to a security issue, XSA-55.

For those concerned about unintentional functional changes, the
following rune produces a version of the patch which is much smaller
and eliminates only non-functional changes:

 GIT_EXTERNAL_DIFF=.../unsigned-differ git-diff <before>..<after>

where <before> and <after> are git refs for the code before and after
this patch, and unsigned-differ is this shell script:

    #!/bin/bash
    set -e

    seddery () {
            perl -pe 's/\b(?:elf_errorstatus|elf_negerrnoval)\b/int/g'
    }

    path="$1"
    in="$2"
    out="$5"

    set +e
    diff -pu --label "$path~" <(seddery <"$in") --label "$path" <(seddery <"$out")
    rc=$?
    set -e
    if [ $rc = 1 ]; then rc=0; fi
    exit $rc

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

v5: Introduce ELF_NOTE_INVALID, instead of using a literal ~0U.

v4: Fix regression in elf_round_up; use uint64_t here.

v3: Changes to booleans split off into separate patch.

v2: BUGFIX: Eliminate conversion to int of return from elf_xen_parse_notes.
    BUGFIX: Fix the one printf format thing which needs changing.
    Remove irrelevant change to constify note_desc.name in libelf-dominfo.c.
    In xc_dom_load_elf_symtab change one sizeof(int) to sizeof(unsigned).
    Do not change type of 2nd argument to memset.
    Provide seddery for easier review.
    Style fix.
---
 tools/libxc/Makefile               |    9 +++++-
 tools/libxc/xc_dom.h               |    7 +++--
 tools/libxc/xc_dom_elfloader.c     |   42 ++++++++++++++++-------------
 tools/xcutils/readnotes.c          |   15 +++++-----
 xen/common/libelf/Makefile         |    2 +
 xen/common/libelf/libelf-dominfo.c |   52 ++++++++++++++++++-----------------
 xen/common/libelf/libelf-loader.c  |   20 +++++++-------
 xen/common/libelf/libelf-tools.c   |   24 ++++++++--------
 xen/include/xen/libelf.h           |   21 ++++++++------
 9 files changed, 105 insertions(+), 87 deletions(-)

diff --git a/tools/libxc/Makefile b/tools/libxc/Makefile
index 4a31282..512a994 100644
--- a/tools/libxc/Makefile
+++ b/tools/libxc/Makefile
@@ -51,8 +51,13 @@ endif
 vpath %.c ../../xen/common/libelf
 CFLAGS += -I../../xen/common/libelf
 
-GUEST_SRCS-y += libelf-tools.c libelf-loader.c
-GUEST_SRCS-y += libelf-dominfo.c
+ELF_SRCS-y += libelf-tools.c libelf-loader.c
+ELF_SRCS-y += libelf-dominfo.c
+
+GUEST_SRCS-y += $(ELF_SRCS-y)
+
+$(patsubst %.c,%.o,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
+$(patsubst %.c,%.opic,$(ELF_SRCS-y)): CFLAGS += -Wno-pointer-sign
 
 # new domain builder
 GUEST_SRCS-y                 += xc_dom_core.c xc_dom_boot.c
diff --git a/tools/libxc/xc_dom.h b/tools/libxc/xc_dom.h
index c7eaf85..567913f 100644
--- a/tools/libxc/xc_dom.h
+++ b/tools/libxc/xc_dom.h
@@ -155,9 +155,10 @@ struct xc_dom_image {
 
 struct xc_dom_loader {
     char *name;
-    int (*probe) (struct xc_dom_image * dom);
-    int (*parser) (struct xc_dom_image * dom);
-    int (*loader) (struct xc_dom_image * dom);
+    /* Sadly the error returns from these functions are not consistent: */
+    elf_negerrnoval (*probe) (struct xc_dom_image * dom);
+    elf_negerrnoval (*parser) (struct xc_dom_image * dom);
+    elf_errorstatus (*loader) (struct xc_dom_image * dom);
 
     struct xc_dom_loader *next;
 };
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 8f9c2fb..eb2e3d2 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -82,7 +82,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
 /* ------------------------------------------------------------------------ */
 /* parse elf binary                                                         */
 
-static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
+static elf_negerrnoval check_elf_kernel(struct xc_dom_image *dom, bool verbose)
 {
     if ( dom->kernel_blob == NULL )
     {
@@ -104,12 +104,12 @@ static int check_elf_kernel(struct xc_dom_image *dom, bool verbose)
     return 0;
 }
 
-static int xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
+static elf_negerrnoval xc_dom_probe_elf_kernel(struct xc_dom_image *dom)
 {
     return check_elf_kernel(dom, 0);
 }
 
-static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
+static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom,
                                   struct elf_binary *elf, bool load)
 {
     struct elf_binary syms;
@@ -117,7 +117,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
     xen_vaddr_t symtab, maxaddr;
     ELF_PTRVAL_CHAR hdr;
     size_t size;
-    int h, count, type, i, tables = 0;
+    unsigned h, count, type, i, tables = 0;
 
     if ( elf_swap(elf) )
     {
@@ -138,13 +138,13 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
         elf->caller_xdest_base = hdr_ptr;
         elf->caller_xdest_size = allow_size;
         hdr = ELF_REALPTR2PTRVAL(hdr_ptr);
-        elf_store_val(elf, int, hdr, size - sizeof(int));
+        elf_store_val(elf, unsigned, hdr, size - sizeof(unsigned));
     }
     else
     {
         char *hdr_ptr;
 
-        size = sizeof(int) + elf_size(elf, elf->ehdr) +
+        size = sizeof(unsigned) + elf_size(elf, elf->ehdr) +
             elf_shdr_count(elf) * elf_size(elf, shdr);
         hdr_ptr = xc_dom_malloc(dom, size);
         if ( hdr_ptr == NULL )
@@ -155,15 +155,15 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
         dom->bsd_symtab_start = elf_round_up(elf, dom->kernel_seg.vend);
     }
 
-    elf_memcpy_safe(elf, hdr + sizeof(int),
+    elf_memcpy_safe(elf, hdr + sizeof(unsigned),
            ELF_IMAGE_BASE(elf),
            elf_size(elf, elf->ehdr));
-    elf_memcpy_safe(elf, hdr + sizeof(int) + elf_size(elf, elf->ehdr),
+    elf_memcpy_safe(elf, hdr + sizeof(unsigned) + elf_size(elf, elf->ehdr),
            ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
            elf_shdr_count(elf) * elf_size(elf, shdr));
     if ( elf_64bit(elf) )
     {
-        Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(int));
+        Elf64_Ehdr *ehdr = (Elf64_Ehdr *)(hdr + sizeof(unsigned));
         ehdr->e_phoff = 0;
         ehdr->e_phentsize = 0;
         ehdr->e_phnum = 0;
@@ -172,22 +172,22 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
     }
     else
     {
-        Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(int));
+        Elf32_Ehdr *ehdr = (Elf32_Ehdr *)(hdr + sizeof(unsigned));
         ehdr->e_phoff = 0;
         ehdr->e_phentsize = 0;
         ehdr->e_phnum = 0;
         ehdr->e_shoff = elf_size(elf, elf->ehdr);
         ehdr->e_shstrndx = SHN_UNDEF;
     }
-    if ( elf->caller_xdest_size < sizeof(int) )
+    if ( elf->caller_xdest_size < sizeof(unsigned) )
     {
         DOMPRINTF("%s/%s: header size %"PRIx64" too small",
                   __FUNCTION__, load ? "load" : "parse",
                   (uint64_t)elf->caller_xdest_size);
         return -1;
     }
-    if ( elf_init(&syms, elf->caller_xdest_base + sizeof(int),
-                  elf->caller_xdest_size - sizeof(int)) )
+    if ( elf_init(&syms, elf->caller_xdest_base + sizeof(unsigned),
+                  elf->caller_xdest_size - sizeof(unsigned)) )
         return -1;
 
     /*
@@ -207,7 +207,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
 
     xc_elf_set_logfile(dom->xch, &syms, 1);
 
-    symtab = dom->bsd_symtab_start + sizeof(int);
+    symtab = dom->bsd_symtab_start + sizeof(unsigned);
     maxaddr = elf_round_up(&syms, symtab + elf_size(&syms, syms.ehdr) +
                            elf_shdr_count(&syms) * elf_size(&syms, shdr));
 
@@ -253,7 +253,7 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
             size = elf_uval(&syms, shdr, sh_size);
             maxaddr = elf_round_up(&syms, maxaddr + size);
             tables++;
-            DOMPRINTF("%s: h=%d %s, size=0x%zx, maxaddr=0x%" PRIx64 "",
+            DOMPRINTF("%s: h=%u %s, size=0x%zx, maxaddr=0x%" PRIx64 "",
                       __FUNCTION__, h,
                       type == SHT_SYMTAB ? "symtab" : "strtab",
                       size, maxaddr);
@@ -292,10 +292,14 @@ static int xc_dom_load_elf_symtab(struct xc_dom_image *dom,
     return 0;
 }
 
-static int xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
+static elf_errorstatus xc_dom_parse_elf_kernel(struct xc_dom_image *dom)
+    /*
+     * This function sometimes returns -1 for error and sometimes
+     * an errno value.  WTF?
+     */
 {
     struct elf_binary *elf;
-    int rc;
+    elf_errorstatus rc;
 
     rc = check_elf_kernel(dom, 1);
     if ( rc != 0 )
@@ -356,10 +360,10 @@ out:
     return rc;
 }
 
-static int xc_dom_load_elf_kernel(struct xc_dom_image *dom)
+static elf_errorstatus xc_dom_load_elf_kernel(struct xc_dom_image *dom)
 {
     struct elf_binary *elf = dom->private_loader;
-    int rc;
+    elf_errorstatus rc;
     xen_pfn_t pages;
 
     elf->dest_base = xc_dom_seg_to_ptr_pages(dom, &dom->kernel_seg, &pages);
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c
index 4394905..88a83fa 100644
--- a/tools/xcutils/readnotes.c
+++ b/tools/xcutils/readnotes.c
@@ -70,7 +70,7 @@ static void print_numeric_note(const char *prefix, struct elf_binary *elf,
 			       ELF_HANDLE_DECL(elf_note) note)
 {
 	uint64_t value = elf_note_numeric(elf, note);
-	int descsz = elf_uval(elf, note, descsz);
+	unsigned descsz = elf_uval(elf, note, descsz);
 
 	printf("%s: %#*" PRIx64 " (%d bytes)\n",
 	       prefix, 2+2*descsz, value, descsz);
@@ -79,7 +79,7 @@ static void print_numeric_note(const char *prefix, struct elf_binary *elf,
 static void print_l1_mfn_valid_note(const char *prefix, struct elf_binary *elf,
 				    ELF_HANDLE_DECL(elf_note) note)
 {
-	int descsz = elf_uval(elf, note, descsz);
+	unsigned descsz = elf_uval(elf, note, descsz);
 	ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
 
 	/* XXX should be able to cope with a list of values. */
@@ -99,10 +99,10 @@ static void print_l1_mfn_valid_note(const char *prefix, struct elf_binary *elf,
 
 }
 
-static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start, ELF_HANDLE_DECL(elf_note) end)
+static unsigned print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start, ELF_HANDLE_DECL(elf_note) end)
 {
 	ELF_HANDLE_DECL(elf_note) note;
-	int notes_found = 0;
+	unsigned notes_found = 0;
 	const char *this_note_name;
 
 	for ( note = start; ELF_HANDLE_PTRVAL(note) < ELF_HANDLE_PTRVAL(end); note = elf_note_next(elf, note) )
@@ -160,7 +160,7 @@ static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start,
 			break;
 		default:
 			printf("unknown note type %#x\n",
-			       (int)elf_uval(elf, note, type));
+			       (unsigned)elf_uval(elf, note, type));
 			break;
 		}
 	}
@@ -170,12 +170,13 @@ static int print_notes(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) start,
 int main(int argc, char **argv)
 {
 	const char *f;
-	int fd,h,size,usize,count;
+	int fd;
+	unsigned h,size,usize,count;
 	void *image,*tmp;
 	struct stat st;
 	struct elf_binary elf;
 	ELF_HANDLE_DECL(elf_shdr) shdr;
-	int notes_found = 0;
+	unsigned notes_found = 0;
 
 	struct setup_header *hdr;
 	uint64_t payload_offset, payload_length;
diff --git a/xen/common/libelf/Makefile b/xen/common/libelf/Makefile
index 18dc8e2..5bf8f76 100644
--- a/xen/common/libelf/Makefile
+++ b/xen/common/libelf/Makefile
@@ -2,6 +2,8 @@ obj-bin-y := libelf.o
 
 SECTIONS := text data $(SPECIAL_DATA_SECTIONS)
 
+CFLAGS += -Wno-pointer-sign
+
 libelf.o: libelf-temp.o Makefile
 	$(OBJCOPY) $(foreach s,$(SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@
 
diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
index c4ced67..0b07002 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -29,15 +29,15 @@ static const char *const elf_xen_feature_names[] = {
     [XENFEAT_pae_pgdir_above_4gb] = "pae_pgdir_above_4gb",
     [XENFEAT_dom0] = "dom0"
 };
-static const int elf_xen_features =
+static const unsigned elf_xen_features =
 sizeof(elf_xen_feature_names) / sizeof(elf_xen_feature_names[0]);
 
-int elf_xen_parse_features(const char *features,
+elf_errorstatus elf_xen_parse_features(const char *features,
                            uint32_t *supported,
                            uint32_t *required)
 {
-    char feature[64];
-    int pos, len, i;
+    unsigned char feature[64];
+    unsigned pos, len, i;
 
     if ( features == NULL )
         return 0;
@@ -94,7 +94,7 @@ int elf_xen_parse_features(const char *features,
 /* ------------------------------------------------------------------------ */
 /* xen elf notes                                                            */
 
-int elf_xen_parse_note(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
                        struct elf_dom_parms *parms,
                        ELF_HANDLE_DECL(elf_note) note)
 {
@@ -125,7 +125,7 @@ int elf_xen_parse_note(struct elf_binary *elf,
     const char *str = NULL;
     uint64_t val = 0;
     unsigned int i;
-    int type = elf_uval(elf, note, type);
+    unsigned type = elf_uval(elf, note, type);
 
     if ( (type >= sizeof(note_desc) / sizeof(note_desc[0])) ||
          (note_desc[type].name == NULL) )
@@ -216,12 +216,14 @@ int elf_xen_parse_note(struct elf_binary *elf,
     return 0;
 }
 
-static int elf_xen_parse_notes(struct elf_binary *elf,
+#define ELF_NOTE_INVALID (~0U)
+
+static unsigned elf_xen_parse_notes(struct elf_binary *elf,
                                struct elf_dom_parms *parms,
                                ELF_PTRVAL_CONST_VOID start,
                                ELF_PTRVAL_CONST_VOID end)
 {
-    int xen_elfnotes = 0;
+    unsigned xen_elfnotes = 0;
     ELF_HANDLE_DECL(elf_note) note;
     const char *note_name;
 
@@ -237,7 +239,7 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
         if ( strcmp(note_name, "Xen") )
             continue;
         if ( elf_xen_parse_note(elf, parms, note) )
-            return -1;
+            return ELF_NOTE_INVALID;
         xen_elfnotes++;
     }
     return xen_elfnotes;
@@ -246,12 +248,12 @@ static int elf_xen_parse_notes(struct elf_binary *elf,
 /* ------------------------------------------------------------------------ */
 /* __xen_guest section                                                      */
 
-int elf_xen_parse_guest_info(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf,
                              struct elf_dom_parms *parms)
 {
     ELF_PTRVAL_CONST_CHAR h;
-    char name[32], value[128];
-    int len;
+    unsigned char name[32], value[128];
+    unsigned len;
 
     h = parms->guest_info;
 #define STAR(h) (elf_access_unsigned(elf, (h), 0, 1))
@@ -334,13 +336,13 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
 /* ------------------------------------------------------------------------ */
 /* sanity checks                                                            */
 
-static int elf_xen_note_check(struct elf_binary *elf,
+static elf_errorstatus elf_xen_note_check(struct elf_binary *elf,
                               struct elf_dom_parms *parms)
 {
     if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) &&
          (ELF_PTRVAL_INVALID(parms->guest_info)) )
     {
-        int machine = elf_uval(elf, elf->ehdr, e_machine);
+        unsigned machine = elf_uval(elf, elf->ehdr, e_machine);
         if ( (machine == EM_386) || (machine == EM_X86_64) )
         {
             elf_err(elf, "%s: ERROR: Not a Xen-ELF image: "
@@ -378,7 +380,7 @@ static int elf_xen_note_check(struct elf_binary *elf,
     return 0;
 }
 
-static int elf_xen_addr_calc_check(struct elf_binary *elf,
+static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf,
                                    struct elf_dom_parms *parms)
 {
     if ( (parms->elf_paddr_offset != UNSET_ADDR) &&
@@ -464,13 +466,13 @@ static int elf_xen_addr_calc_check(struct elf_binary *elf,
 /* ------------------------------------------------------------------------ */
 /* glue it all together ...                                                 */
 
-int elf_xen_parse(struct elf_binary *elf,
+elf_errorstatus elf_xen_parse(struct elf_binary *elf,
                   struct elf_dom_parms *parms)
 {
     ELF_HANDLE_DECL(elf_shdr) shdr;
     ELF_HANDLE_DECL(elf_phdr) phdr;
-    int xen_elfnotes = 0;
-    int i, count, rc;
+    unsigned xen_elfnotes = 0;
+    unsigned i, count, more_notes;
 
     elf_memset_unchecked(parms, 0, sizeof(*parms));
     parms->virt_base = UNSET_ADDR;
@@ -495,13 +497,13 @@ int elf_xen_parse(struct elf_binary *elf,
         if (elf_uval(elf, phdr, p_offset) == 0)
              continue;
 
-        rc = elf_xen_parse_notes(elf, parms,
+        more_notes = elf_xen_parse_notes(elf, parms,
                                  elf_segment_start(elf, phdr),
                                  elf_segment_end(elf, phdr));
-        if ( rc == -1 )
+        if ( more_notes == ELF_NOTE_INVALID )
             return -1;
 
-        xen_elfnotes += rc;
+        xen_elfnotes += more_notes;
     }
 
     /*
@@ -518,17 +520,17 @@ int elf_xen_parse(struct elf_binary *elf,
             if ( elf_uval(elf, shdr, sh_type) != SHT_NOTE )
                 continue;
 
-            rc = elf_xen_parse_notes(elf, parms,
+            more_notes = elf_xen_parse_notes(elf, parms,
                                      elf_section_start(elf, shdr),
                                      elf_section_end(elf, shdr));
 
-            if ( rc == -1 )
+            if ( more_notes == ELF_NOTE_INVALID )
                 return -1;
 
-            if ( xen_elfnotes == 0 && rc > 0 )
+            if ( xen_elfnotes == 0 && more_notes > 0 )
                 elf_msg(elf, "%s: using notes from SHT_NOTE section\n", __FUNCTION__);
 
-            xen_elfnotes += rc;
+            xen_elfnotes += more_notes;
         }
 
     }
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index 798f88b..937c99b 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -24,7 +24,7 @@
 
 /* ------------------------------------------------------------------------ */
 
-int elf_init(struct elf_binary *elf, const char *image_input, size_t size)
+elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t size)
 {
     ELF_HANDLE_DECL(elf_shdr) shdr;
     uint64_t i, count, section, offset;
@@ -114,7 +114,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback *log_callback,
     elf->verbose = verbose;
 }
 
-static int elf_load_image(struct elf_binary *elf,
+static elf_errorstatus elf_load_image(struct elf_binary *elf,
                           ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src,
                           uint64_t filesz, uint64_t memsz)
 {
@@ -129,9 +129,9 @@ void elf_set_verbose(struct elf_binary *elf)
     elf->verbose = 1;
 }
 
-static int elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
+static elf_errorstatus elf_load_image(struct elf_binary *elf, ELF_PTRVAL_VOID dst, ELF_PTRVAL_CONST_VOID src, uint64_t filesz, uint64_t memsz)
 {
-    int rc;
+    elf_errorstatus rc;
     if ( filesz > ULONG_MAX || memsz > ULONG_MAX )
         return -1;
     /* We trust the dom0 kernel image completely, so we don't care
@@ -151,7 +151,7 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t pstart)
 {
     uint64_t sz;
     ELF_HANDLE_DECL(elf_shdr) shdr;
-    int i, type;
+    unsigned i, type;
 
     if ( !ELF_HANDLE_VALID(elf->sym_tab) )
         return;
@@ -187,7 +187,7 @@ static void elf_load_bsdsyms(struct elf_binary *elf)
     ELF_PTRVAL_VOID symbase;
     ELF_PTRVAL_VOID symtab_addr;
     ELF_HANDLE_DECL_NONCONST(elf_shdr) shdr;
-    int i, type;
+    unsigned i, type;
 
     if ( !elf->bsd_symtab_pstart )
         return;
@@ -220,7 +220,7 @@ do {                                            \
     elf_memcpy_safe(elf, ELF_HANDLE_PTRVAL(shdr),
                     ELF_IMAGE_BASE(elf) + elf_uval(elf, elf->ehdr, e_shoff),
                     sz);
-    maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
+    maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva + sz);
 
     for ( i = 0; i < elf_shdr_count(elf); i++ )
     {
@@ -233,10 +233,10 @@ do {                                            \
              elf_memcpy_safe(elf, maxva, elf_section_start(elf, shdr), sz);
              /* Mangled to be based on ELF header location. */
              elf_hdr_elm(elf, shdr, sh_offset, maxva - symtab_addr);
-             maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (long)maxva + sz);
+             maxva = ELF_OBSOLETE_VOIDP_CAST elf_round_up(elf, (unsigned long)maxva + sz);
         }
         shdr = ELF_MAKE_HANDLE(elf_shdr, ELF_HANDLE_PTRVAL(shdr) +
-                            (long)elf_uval(elf, elf->ehdr, e_shentsize));
+                            (unsigned long)elf_uval(elf, elf->ehdr, e_shentsize));
     }
 
     /* Write down the actual sym size. */
@@ -273,7 +273,7 @@ void elf_parse_binary(struct elf_binary *elf)
             __FUNCTION__, elf->pstart, elf->pend);
 }
 
-int elf_load_binary(struct elf_binary *elf)
+elf_errorstatus elf_load_binary(struct elf_binary *elf)
 {
     ELF_HANDLE_DECL(elf_phdr) phdr;
     uint64_t i, count, paddr, offset, filesz, memsz;
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 0b7b2b6..6543f33 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -122,19 +122,19 @@ uint64_t elf_access_unsigned(struct elf_binary * elf, elf_ptrval base,
 
 uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr)
 {
-    int elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
+    uint64_t elf_round = (elf_64bit(elf) ? 8 : 4) - 1;
 
     return (addr + elf_round) & ~elf_round;
 }
 
 /* ------------------------------------------------------------------------ */
 
-int elf_shdr_count(struct elf_binary *elf)
+unsigned elf_shdr_count(struct elf_binary *elf)
 {
     return elf_uval(elf, elf->ehdr, e_shnum);
 }
 
-int elf_phdr_count(struct elf_binary *elf)
+unsigned elf_phdr_count(struct elf_binary *elf)
 {
     return elf_uval(elf, elf->ehdr, e_phnum);
 }
@@ -144,7 +144,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *n
     uint64_t count = elf_shdr_count(elf);
     ELF_HANDLE_DECL(elf_shdr) shdr;
     const char *sname;
-    int i;
+    unsigned i;
 
     for ( i = 0; i < count; i++ )
     {
@@ -156,7 +156,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *n
     return ELF_INVALID_HANDLE(elf_shdr);
 }
 
-ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned index)
 {
     uint64_t count = elf_shdr_count(elf);
     ELF_PTRVAL_CONST_VOID ptr;
@@ -170,7 +170,7 @@ ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index)
     return ELF_MAKE_HANDLE(elf_shdr, ptr);
 }
 
-ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned index)
 {
     uint64_t count = elf_uval(elf, elf->ehdr, e_phnum);
     ELF_PTRVAL_CONST_VOID ptr;
@@ -264,7 +264,7 @@ ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char *sym
     return ELF_INVALID_HANDLE(elf_sym);
 }
 
-ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index)
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned index)
 {
     ELF_PTRVAL_CONST_VOID ptr = elf_section_start(elf, elf->sym_tab);
     ELF_HANDLE_DECL(elf_sym) sym;
@@ -280,7 +280,7 @@ const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note
 
 ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
 {
-    int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
+    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
 
     return ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz;
 }
@@ -288,7 +288,7 @@ ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_
 uint64_t elf_note_numeric(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
 {
     ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
-    int descsz = elf_uval(elf, note, descsz);
+    unsigned descsz = elf_uval(elf, note, descsz);
 
     switch (descsz)
     {
@@ -306,7 +306,7 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note
                                 unsigned int unitsz, unsigned int idx)
 {
     ELF_PTRVAL_CONST_VOID desc = elf_note_desc(elf, note);
-    int descsz = elf_uval(elf, note, descsz);
+    unsigned descsz = elf_uval(elf, note, descsz);
 
     if ( descsz % unitsz || idx >= descsz / unitsz )
         return 0;
@@ -324,8 +324,8 @@ uint64_t elf_note_numeric_array(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note
 
 ELF_HANDLE_DECL(elf_note) elf_note_next(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note)
 {
-    int namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
-    int descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
+    unsigned namesz = (elf_uval(elf, note, namesz) + 3) & ~3;
+    unsigned descsz = (elf_uval(elf, note, descsz) + 3) & ~3;
 
     return ELF_MAKE_HANDLE(elf_note, ELF_HANDLE_PTRVAL(note) + elf_size(elf, note) + namesz + descsz);
 }
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 1fa7421..e4588de 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -31,6 +31,9 @@
 
 #include <stdbool.h>
 
+typedef int elf_errorstatus; /* 0: ok; -ve (normally -1): error */
+typedef int elf_negerrnoval; /* 0: ok; -EFOO: error */
+
 #undef ELFSIZE
 #include "elfstructs.h"
 #ifdef __XEN__
@@ -327,12 +330,12 @@ bool elf_access_ok(struct elf_binary * elf,
 /* ------------------------------------------------------------------------ */
 /* xc_libelf_tools.c                                                        */
 
-int elf_shdr_count(struct elf_binary *elf);
-int elf_phdr_count(struct elf_binary *elf);
+unsigned elf_shdr_count(struct elf_binary *elf);
+unsigned elf_phdr_count(struct elf_binary *elf);
 
 ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_name(struct elf_binary *elf, const char *name);
-ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, int index);
-ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_shdr) elf_shdr_by_index(struct elf_binary *elf, unsigned index);
+ELF_HANDLE_DECL(elf_phdr) elf_phdr_by_index(struct elf_binary *elf, unsigned index);
 
 const char *elf_section_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr) shdr); /* might return NULL if inputs are invalid */
 ELF_PTRVAL_CONST_VOID elf_section_start(struct elf_binary *elf, ELF_HANDLE_DECL(elf_shdr) shdr);
@@ -342,7 +345,7 @@ ELF_PTRVAL_CONST_VOID elf_segment_start(struct elf_binary *elf, ELF_HANDLE_DECL(
 ELF_PTRVAL_CONST_VOID elf_segment_end(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr);
 
 ELF_HANDLE_DECL(elf_sym) elf_sym_by_name(struct elf_binary *elf, const char *symbol);
-ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, int index);
+ELF_HANDLE_DECL(elf_sym) elf_sym_by_index(struct elf_binary *elf, unsigned index);
 
 const char *elf_note_name(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note); /* may return NULL */
 ELF_PTRVAL_CONST_VOID elf_note_desc(struct elf_binary *elf, ELF_HANDLE_DECL(elf_note) note);
@@ -357,7 +360,7 @@ bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr
 /* ------------------------------------------------------------------------ */
 /* xc_libelf_loader.c                                                       */
 
-int elf_init(struct elf_binary *elf, const char *image, size_t size);
+elf_errorstatus elf_init(struct elf_binary *elf, const char *image, size_t size);
   /*
    * image and size must be correct.  They will be recorded in
    * *elf, and must remain valid while the elf is in use.
@@ -370,7 +373,7 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback*,
 #endif
 
 void elf_parse_binary(struct elf_binary *elf);
-int elf_load_binary(struct elf_binary *elf);
+elf_errorstatus elf_load_binary(struct elf_binary *elf);
 
 ELF_PTRVAL_VOID elf_get_ptr(struct elf_binary *elf, unsigned long addr);
 uint64_t elf_lookup_addr(struct elf_binary *elf, const char *symbol);
@@ -383,7 +386,7 @@ const char *elf_check_broken(const struct elf_binary *elf); /* NULL means OK */
 /* ------------------------------------------------------------------------ */
 /* xc_libelf_relocate.c                                                     */
 
-int elf_reloc(struct elf_binary *elf);
+elf_errorstatus elf_reloc(struct elf_binary *elf);
 
 /* ------------------------------------------------------------------------ */
 /* xc_libelf_dominfo.c                                                      */
@@ -417,7 +420,7 @@ struct elf_dom_parms {
     char guest_ver[16];
     char xen_ver[16];
     char loader[16];
-    int pae;
+    int pae; /* some kind of enum apparently */
     bool bsd_symtab;
     uint64_t virt_base;
     uint64_t virt_entry;
-- 
1.7.2.5

  parent reply	other threads:[~2013-06-07 18:27 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-07 18:27 [PATCH v6 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-07 18:27 ` [PATCH 01/22] libelf: abolish libelf-relocate.c Ian Jackson
2013-06-07 18:27 ` [PATCH 02/22] libxc: introduce xc_dom_seg_to_ptr_pages Ian Jackson
2013-06-10 12:21   ` Andrew Cooper
2013-06-10 13:40     ` Ian Jackson
2013-06-10 13:49       ` Andrew Cooper
2013-06-10 14:02         ` Ian Jackson
2013-06-10 15:24           ` Andrew Cooper
2013-06-07 18:27 ` [PATCH 03/22] libxc: Fix range checking in xc_dom_pfn_to_ptr etc Ian Jackson
2013-06-10 15:46   ` Andrew Cooper
2013-06-10 15:48     ` Ian Jackson
2013-06-10 15:53       ` Andrew Cooper
2013-06-10 15:54         ` Ian Jackson
2013-06-07 18:27 ` [PATCH 04/22] libelf: add `struct elf_binary*' parameter to elf_load_image Ian Jackson
2013-06-07 18:27 ` [PATCH 05/22] libelf: abolish elf_sval and elf_access_signed Ian Jackson
2013-06-07 18:27 ` [PATCH 06/22] libelf: move include of <asm/guest_access.h> to top of file Ian Jackson
2013-06-07 18:27 ` [PATCH 07/22] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised Ian Jackson
2013-06-07 18:27 ` [PATCH 08/22] libelf: introduce macros for memory access and pointer handling Ian Jackson
2013-06-10 16:57   ` Andrew Cooper
2013-06-10 16:58     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 09/22] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Ian Jackson
2013-06-07 18:27 ` [PATCH 10/22] libelf: check nul-terminated strings properly Ian Jackson
2013-06-10 23:17   ` Andrew Cooper
2013-06-11 15:17     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 11/22] libelf: check all pointer accesses Ian Jackson
2013-06-10 23:38   ` Andrew Cooper
2013-06-11 15:28     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 12/22] libelf: Check pointer references in elf_is_elfbinary Ian Jackson
2013-06-10 23:04   ` Andrew Cooper
2013-06-11 15:11     ` Ian Jackson
2013-06-11 16:00       ` Andrew Cooper
2013-06-11 16:33         ` Ian Jackson
2013-06-07 18:27 ` [PATCH 13/22] libelf: Make all callers call elf_check_broken Ian Jackson
2013-06-07 18:27 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-10 22:09   ` Andrew Cooper
2013-06-11  7:17     ` Jan Beulich
2013-06-11 14:50     ` Ian Jackson
2013-06-07 18:27 ` Ian Jackson [this message]
2013-06-10 18:28   ` [PATCH 15/22] libelf: use only unsigned integers Andrew Cooper
2013-06-11 13:16     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 16/22] libelf: check loops for running away Ian Jackson
2013-06-10 22:56   ` Andrew Cooper
2013-06-11 15:07     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 17/22] libelf: abolish obsolete macros Ian Jackson
2013-06-07 18:27 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-10 21:37   ` Andrew Cooper
2013-06-11 14:46     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range Ian Jackson
2013-06-10 21:09   ` Andrew Cooper
2013-06-11 14:44     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-07 18:32   ` Andrew Cooper
2013-06-11 15:53     ` Ian Jackson
2013-06-10 20:38   ` Andrew Cooper
2013-06-11 14:31     ` Ian Jackson
2013-06-07 18:27 ` [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest Ian Jackson
2013-06-07 18:33   ` Andrew Cooper
2013-06-08 12:05     ` Andrew Cooper
2013-06-11 15:58       ` Ian Jackson
2013-06-11 16:19         ` Tim Deegan
2013-06-07 18:27 ` [PATCH 22/22] libxc: check blob size before proceeding in xc_dom_check_gzip Ian Jackson
2013-06-10 20:10   ` Andrew Cooper
2013-06-11 13:46     ` Ian Jackson
2013-06-10 23:44 ` [PATCH v6 00/22] XSA55 libelf fixes for unstable Andrew Cooper
  -- strict thread matches above, loose matches on Subject: below --
2013-06-07 18:35 [PATCH v6 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-07 18:35 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson
2013-06-11 18:20 [PATCH v7 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-11 18:20 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson
2013-06-11 19:22   ` Andrew Cooper
2013-06-11 18:22 [PATCH v7 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-11 18:22 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1370629642-6990-16-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=mattjd@gmail.com \
    --cc=security@xen.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).