diff options
author | Ian Jackson <ian.jackson@eu.citrix.com> | 2013-06-14 16:39:36 +0100 |
---|---|---|
committer | Ian Jackson <Ian.Jackson@eu.citrix.com> | 2013-06-14 16:39:36 +0100 |
commit | a004800f8fc607b96527815c8e3beabcb455d8e0 (patch) | |
tree | bedfb7dc80066010059b5ffd17b8affdb138b8cd /tools/xcutils | |
parent | 7a549a6aa04dba807f8dd4c1577ab6a7592c4c76 (diff) | |
download | xen-a004800f8fc607b96527815c8e3beabcb455d8e0.tar.gz xen-a004800f8fc607b96527815c8e3beabcb455d8e0.tar.bz2 xen-a004800f8fc607b96527815c8e3beabcb455d8e0.zip |
libelf: use only unsigned integers
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: George Dunlap <george.dunlap@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
v8: Use "?!?!" to express consternation instead of a ruder phrase.
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.
Diffstat (limited to 'tools/xcutils')
-rw-r--r-- | tools/xcutils/readnotes.c | 15 |
1 files changed, 8 insertions, 7 deletions
diff --git a/tools/xcutils/readnotes.c b/tools/xcutils/readnotes.c index d1f7a30f73..2ca7732e2d 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; |