| Message ID | 1295191700-5312-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Accepted |
| Headers | show |
Thanks, applied. On Sun, Jan 16, 2011 at 3:28 PM, Stefan Weil <weil@mail.berlios.de> wrote: > These errors were reported by cppcheck: > > [bsd-user/elfload.c:1108]: (error) Common realloc mistake: "syms" nulled but not freed upon failure > [bsd-user/elfload.c:1076]: (error) Memory leak: s > [bsd-user/elfload.c:1079]: (error) Memory leak: syms > > v2: > * The previous fix for memory leaks was incomplete (thanks to Peter Maydell for te hint). > * Fix wrong realloc usage, too. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > bsd-user/elfload.c | 37 +++++++++++++++++++++++++++++++------ > 1 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c > index 7374912..1ef1f97 100644 > --- a/bsd-user/elfload.c > +++ b/bsd-user/elfload.c > @@ -1044,7 +1044,7 @@ static void load_symbols(struct elfhdr *hdr, int fd) > struct elf_shdr sechdr, symtab, strtab; > char *strings; > struct syminfo *s; > - struct elf_sym *syms; > + struct elf_sym *syms, *new_syms; > > lseek(fd, hdr->e_shoff, SEEK_SET); > for (i = 0; i < hdr->e_shnum; i++) { > @@ -1072,15 +1072,24 @@ static void load_symbols(struct elfhdr *hdr, int fd) > /* Now know where the strtab and symtab are. Snarf them. */ > s = malloc(sizeof(*s)); > syms = malloc(symtab.sh_size); > - if (!syms) > + if (!syms) { > + free(s); > return; > + } > s->disas_strtab = strings = malloc(strtab.sh_size); > - if (!s->disas_strtab) > + if (!s->disas_strtab) { > + free(s); > + free(syms); > return; > + } > > lseek(fd, symtab.sh_offset, SEEK_SET); > - if (read(fd, syms, symtab.sh_size) != symtab.sh_size) > + if (read(fd, syms, symtab.sh_size) != symtab.sh_size) { > + free(s); > + free(syms); > + free(strings); > return; > + } > > nsyms = symtab.sh_size / sizeof(struct elf_sym); > > @@ -1105,13 +1114,29 @@ static void load_symbols(struct elfhdr *hdr, int fd) > #endif > i++; > } > - syms = realloc(syms, nsyms * sizeof(*syms)); > + > + /* Attempt to free the storage associated with the local symbols > + that we threw away. Whether or not this has any effect on the > + memory allocation depends on the malloc implementation and how > + many symbols we managed to discard. */ > + new_syms = realloc(syms, nsyms * sizeof(*syms)); > + if (new_syms == NULL) { > + free(s); > + free(syms); > + free(strings); > + return; > + } > + syms = new_syms; > > qsort(syms, nsyms, sizeof(*syms), symcmp); > > lseek(fd, strtab.sh_offset, SEEK_SET); > - if (read(fd, strings, strtab.sh_size) != strtab.sh_size) > + if (read(fd, strings, strtab.sh_size) != strtab.sh_size) { > + free(s); > + free(syms); > + free(strings); > return; > + } > s->disas_num_syms = nsyms; > #if ELF_CLASS == ELFCLASS32 > s->disas_symtab.elf32 = syms; > -- > 1.7.2.3 > >
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c index 7374912..1ef1f97 100644 --- a/bsd-user/elfload.c +++ b/bsd-user/elfload.c @@ -1044,7 +1044,7 @@ static void load_symbols(struct elfhdr *hdr, int fd) struct elf_shdr sechdr, symtab, strtab; char *strings; struct syminfo *s; - struct elf_sym *syms; + struct elf_sym *syms, *new_syms; lseek(fd, hdr->e_shoff, SEEK_SET); for (i = 0; i < hdr->e_shnum; i++) { @@ -1072,15 +1072,24 @@ static void load_symbols(struct elfhdr *hdr, int fd) /* Now know where the strtab and symtab are. Snarf them. */ s = malloc(sizeof(*s)); syms = malloc(symtab.sh_size); - if (!syms) + if (!syms) { + free(s); return; + } s->disas_strtab = strings = malloc(strtab.sh_size); - if (!s->disas_strtab) + if (!s->disas_strtab) { + free(s); + free(syms); return; + } lseek(fd, symtab.sh_offset, SEEK_SET); - if (read(fd, syms, symtab.sh_size) != symtab.sh_size) + if (read(fd, syms, symtab.sh_size) != symtab.sh_size) { + free(s); + free(syms); + free(strings); return; + } nsyms = symtab.sh_size / sizeof(struct elf_sym); @@ -1105,13 +1114,29 @@ static void load_symbols(struct elfhdr *hdr, int fd) #endif i++; } - syms = realloc(syms, nsyms * sizeof(*syms)); + + /* Attempt to free the storage associated with the local symbols + that we threw away. Whether or not this has any effect on the + memory allocation depends on the malloc implementation and how + many symbols we managed to discard. */ + new_syms = realloc(syms, nsyms * sizeof(*syms)); + if (new_syms == NULL) { + free(s); + free(syms); + free(strings); + return; + } + syms = new_syms; qsort(syms, nsyms, sizeof(*syms), symcmp); lseek(fd, strtab.sh_offset, SEEK_SET); - if (read(fd, strings, strtab.sh_size) != strtab.sh_size) + if (read(fd, strings, strtab.sh_size) != strtab.sh_size) { + free(s); + free(syms); + free(strings); return; + } s->disas_num_syms = nsyms; #if ELF_CLASS == ELFCLASS32 s->disas_symtab.elf32 = syms;
These errors were reported by cppcheck: [bsd-user/elfload.c:1108]: (error) Common realloc mistake: "syms" nulled but not freed upon failure [bsd-user/elfload.c:1076]: (error) Memory leak: s [bsd-user/elfload.c:1079]: (error) Memory leak: syms v2: * The previous fix for memory leaks was incomplete (thanks to Peter Maydell for te hint). * Fix wrong realloc usage, too. Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- bsd-user/elfload.c | 37 +++++++++++++++++++++++++++++++------ 1 files changed, 31 insertions(+), 6 deletions(-)