aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2017-02-04 23:05:33 +0000
committerLaurent Vivier <laurent@vivier.eu>2017-02-16 15:29:30 +0100
commit1e06262da615fcc0ddd658f96c5673a73b856fb6 (patch)
treefa9b132e6642534c046b7e9bc0e98f3156a184a6
parent26920a2961f7cc86bfbdb2184c0ec261d5629c2f (diff)
downloadqemu-1e06262da615fcc0ddd658f96c5673a73b856fb6.zip
qemu-1e06262da615fcc0ddd658f96c5673a73b856fb6.tar.gz
qemu-1e06262da615fcc0ddd658f96c5673a73b856fb6.tar.bz2
linux-user: Use correct types in load_symbols()
Coverity doesn't like the code in load_symbols() which assumes it can use 'int' for a variable that might hold an offset into the guest ELF file, because in a 64-bit guest that could overflow. Guest binaries with 2GB sections aren't very likely and this isn't a security issue because we fully trust the guest linux-user binary anyway, but we might as well use the right types, which will placate Coverity. Use uint64_t to hold section sizes, and bail out if the symbol table is too large rather than just overflowing an int. (Coverity issue CID1005776) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <1486249533-5260-1-git-send-email-peter.maydell@linaro.org> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
-rw-r--r--linux-user/elfload.c22
1 files changed, 15 insertions, 7 deletions
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 8271227..f520d77 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2262,6 +2262,7 @@ static int symcmp(const void *s0, const void *s1)
static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias)
{
int i, shnum, nsyms, sym_idx = 0, str_idx = 0;
+ uint64_t segsz;
struct elf_shdr *shdr;
char *strings = NULL;
struct syminfo *s = NULL;
@@ -2293,19 +2294,26 @@ static void load_symbols(struct elfhdr *hdr, int fd, abi_ulong load_bias)
goto give_up;
}
- i = shdr[str_idx].sh_size;
- s->disas_strtab = strings = g_try_malloc(i);
- if (!strings || pread(fd, strings, i, shdr[str_idx].sh_offset) != i) {
+ segsz = shdr[str_idx].sh_size;
+ s->disas_strtab = strings = g_try_malloc(segsz);
+ if (!strings ||
+ pread(fd, strings, segsz, shdr[str_idx].sh_offset) != segsz) {
goto give_up;
}
- i = shdr[sym_idx].sh_size;
- syms = g_try_malloc(i);
- if (!syms || pread(fd, syms, i, shdr[sym_idx].sh_offset) != i) {
+ segsz = shdr[sym_idx].sh_size;
+ syms = g_try_malloc(segsz);
+ if (!syms || pread(fd, syms, segsz, shdr[sym_idx].sh_offset) != segsz) {
goto give_up;
}
- nsyms = i / sizeof(struct elf_sym);
+ if (segsz / sizeof(struct elf_sym) > INT_MAX) {
+ /* Implausibly large symbol table: give up rather than ploughing
+ * on with the number of symbols calculation overflowing
+ */
+ goto give_up;
+ }
+ nsyms = segsz / sizeof(struct elf_sym);
for (i = 0; i < nsyms; ) {
bswap_sym(syms + i);
/* Throw away entries which we do not need. */