OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

asan: more readelf leaks

Browse Source 
* elfcomm.c (get_archive_member_name): Always return malloc'd
	string or NULL.
	* elfedit.c (process_archive): Tidy memory on all return paths.
	* readelf.c (process_archive): Likewise.
	(process_symbol_table): Likewise.
	(ba_cache): New, replacing ..
	(get_symbol_for_build_attribute): ..static vars here.  Free
	strtab and symtab before loading new ones.  Reject symtab without
	valid strtab in loop, breaking out of loop on valid symtab.
	(process_file): Free ba_cache symtab and strtab here, resetting
	ba_cache.
This commit is contained in:
Alan Modra 2020-03-13 13:21:15 +10:30
parent 49ba92c0a6
commit fd486f32d1
4 changed files with 120 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
binutils/ChangeLog
View File

@ -1,3 +1,17 @@
2020-03-13 Alan Modra <amodra@gmail.com>
* elfcomm.c (get_archive_member_name): Always return malloc'd
string or NULL.
* elfedit.c (process_archive): Tidy memory on all return paths.
* readelf.c (process_archive): Likewise.
(process_symbol_table): Likewise.
(ba_cache): New, replacing ..
(get_symbol_for_build_attribute): ..static vars here. Free
strtab and symtab before loading new ones. Reject symtab without
valid strtab in loop, breaking out of loop on valid symtab.
(process_file): Free ba_cache symtab and strtab here, resetting
ba_cache.
2020-03-12 Alan Modra <amodra@gmail.com> 2020-03-12 Alan Modra <amodra@gmail.com>
* readelf.c (process_section_headers): Don't just set * readelf.c (process_section_headers): Don't just set

8
binutils/elfcomm.c
View File

@ -797,7 +797,7 @@ get_archive_member_name (struct archive_info *arch,
arch->longnames[j] = '\0'; arch->longnames[j] = '\0';
if (!arch->is_thin_archive || arch->nested_member_origin == 0) if (!arch->is_thin_archive || arch->nested_member_origin == 0)
return arch->longnames + k; return xstrdup (arch->longnames + k);
/* PR 17531: file: 2896dc8b. */ /* PR 17531: file: 2896dc8b. */
if (k >= j) if (k >= j)
@ -813,7 +813,7 @@ get_archive_member_name (struct archive_info *arch,
if (member_file_name != NULL if (member_file_name != NULL
&& setup_nested_archive (nested_arch, member_file_name) == 0) && setup_nested_archive (nested_arch, member_file_name) == 0)
{ {
member_name = get_archive_member_name_at (nested_arch, member_name = get_archive_member_name_at (nested_arch,
arch->nested_member_origin, arch->nested_member_origin,
NULL); NULL);
if (member_name != NULL) if (member_name != NULL)
@ -825,7 +825,7 @@ get_archive_member_name (struct archive_info *arch,
free (member_file_name); free (member_file_name);
/* Last resort: just return the name of the nested archive. */ /* Last resort: just return the name of the nested archive. */
return arch->longnames + k; return xstrdup (arch->longnames + k);
} }
/* We have a normal (short) name. */ /* We have a normal (short) name. */
@ -833,7 +833,7 @@ get_archive_member_name (struct archive_info *arch,
if (arch->arhdr.ar_name[j] == '/') if (arch->arhdr.ar_name[j] == '/')
{ {
arch->arhdr.ar_name[j] = '\0'; arch->arhdr.ar_name[j] = '\0';
return arch->arhdr.ar_name; return xstrdup (arch->arhdr.ar_name);
} }
/* The full ar_name field is used. Don't rely on ar_date starting /* The full ar_name field is used. Don't rely on ar_date starting

9
binutils/elfedit.c
View File

@ -616,6 +616,7 @@ process_archive (const char * file_name, FILE * file,
if (qualified_name == NULL) if (qualified_name == NULL)
{ {
error (_("%s: bad archive file name\n"), file_name); error (_("%s: bad archive file name\n"), file_name);
free (name);
ret = 1; ret = 1;
break; break;
} }
@ -626,8 +627,10 @@ process_archive (const char * file_name, FILE * file,
FILE *member_file; FILE *member_file;
char *member_file_name = adjust_relative_path (file_name, char *member_file_name = adjust_relative_path (file_name,
name, namelen); name, namelen);
free (name);
if (member_file_name == NULL) if (member_file_name == NULL)
{ {
free (qualified_name);
ret = 1; ret = 1;
break; break;
} }
@ -638,6 +641,7 @@ process_archive (const char * file_name, FILE * file,
error (_("Input file '%s' is not readable\n"), error (_("Input file '%s' is not readable\n"),
member_file_name); member_file_name);
free (member_file_name); free (member_file_name);
free (qualified_name);
ret = 1; ret = 1;
break; break;
} }
@ -648,9 +652,12 @@ process_archive (const char * file_name, FILE * file,
fclose (member_file); fclose (member_file);
free (member_file_name); free (member_file_name);
free (qualified_name);
} }
else if (is_thin_archive) else if (is_thin_archive)
{ {
free (name);
/* This is a proxy for a member of a nested archive. */ /* This is a proxy for a member of a nested archive. */
archive_file_offset = arch.nested_member_origin + sizeof arch.arhdr; archive_file_offset = arch.nested_member_origin + sizeof arch.arhdr;
@ -661,6 +668,7 @@ process_archive (const char * file_name, FILE * file,
{ {
error (_("%s: failed to seek to archive member\n"), error (_("%s: failed to seek to archive member\n"),
nested_arch.file_name); nested_arch.file_name);
free (qualified_name);
ret = 1; ret = 1;
break; break;
} }
@ -669,6 +677,7 @@ process_archive (const char * file_name, FILE * file,
} }
else else
{ {
free (name);
archive_file_offset = arch.next_arhdr_offset; archive_file_offset = arch.next_arhdr_offset;
arch.next_arhdr_offset += archive_file_size; arch.next_arhdr_offset += archive_file_size;

150
binutils/readelf.c
View File

@ -11766,17 +11766,17 @@ process_symbol_table (Filedata * filedata)
buckets = get_dynamic_data (filedata, nbuckets, hash_ent_size); buckets = get_dynamic_data (filedata, nbuckets, hash_ent_size);
chains = get_dynamic_data (filedata, nchains, hash_ent_size); chains = get_dynamic_data (filedata, nchains, hash_ent_size);
no_hash:
if (buckets == NULL || chains == NULL) if (buckets == NULL || chains == NULL)
{ {
if (do_using_dynamic) no_hash:
return FALSE;
free (buckets); free (buckets);
free (chains); free (chains);
buckets = NULL; buckets = NULL;
chains = NULL; chains = NULL;
nbuckets = 0; nbuckets = 0;
nchains = 0; nchains = 0;
if (do_using_dynamic)
goto err_out;
} }
} }
@ -11833,7 +11833,7 @@ process_symbol_table (Filedata * filedata)
if (gnubuckets[i] != 0) if (gnubuckets[i] != 0)
{ {
if (gnubuckets[i] < gnusymidx) if (gnubuckets[i] < gnusymidx)
return FALSE; goto err_out;
if (maxchain == 0xffffffff || gnubuckets[i] > maxchain) if (maxchain == 0xffffffff || gnubuckets[i] > maxchain)
maxchain = gnubuckets[i]; maxchain = gnubuckets[i];
@ -11898,21 +11898,17 @@ process_symbol_table (Filedata * filedata)
} }
mipsxlat = get_dynamic_data (filedata, maxchain, 4); mipsxlat = get_dynamic_data (filedata, maxchain, 4);
} if (mipsxlat == NULL)
{
no_gnu_hash: no_gnu_hash:
if (dynamic_info_DT_MIPS_XHASH && mipsxlat == NULL) free (gnuchains);
{ gnuchains = NULL;
free (gnuchains); free (gnubuckets);
gnuchains = NULL; gnubuckets = NULL;
} ngnubuckets = 0;
if (gnuchains == NULL) if (do_using_dynamic)
{ goto err_out;
free (gnubuckets); }
gnubuckets = NULL;
ngnubuckets = 0;
if (do_using_dynamic)
return FALSE;
} }
} }
@ -12129,7 +12125,7 @@ process_symbol_table (Filedata * filedata)
if (lengths == NULL) if (lengths == NULL)
{ {
error (_("Out of memory allocating space for histogram buckets\n")); error (_("Out of memory allocating space for histogram buckets\n"));
return FALSE; goto err_out;
} }
visited = xcmalloc (nchains, 1); visited = xcmalloc (nchains, 1);
memset (visited, 0, nchains); memset (visited, 0, nchains);
@ -12157,7 +12153,7 @@ process_symbol_table (Filedata * filedata)
{ {
free (lengths); free (lengths);
error (_("Out of memory allocating space for histogram counts\n")); error (_("Out of memory allocating space for histogram counts\n"));
return FALSE; goto err_out;
} }
for (hn = 0; hn < nbuckets; ++hn) for (hn = 0; hn < nbuckets; ++hn)
@ -12181,11 +12177,10 @@ process_symbol_table (Filedata * filedata)
free (lengths); free (lengths);
} }
if (buckets != NULL) free (buckets);
{ buckets = NULL;
free (buckets); free (chains);
free (chains); chains = NULL;
}
if (do_histogram && gnubuckets != NULL) if (do_histogram && gnubuckets != NULL)
{ {
@ -12208,7 +12203,7 @@ process_symbol_table (Filedata * filedata)
if (lengths == NULL) if (lengths == NULL)
{ {
error (_("Out of memory allocating space for gnu histogram buckets\n")); error (_("Out of memory allocating space for gnu histogram buckets\n"));
return FALSE; goto err_out;
} }
printf (_(" Length Number %% of total Coverage\n")); printf (_(" Length Number %% of total Coverage\n"));
@ -12234,7 +12229,7 @@ process_symbol_table (Filedata * filedata)
{ {
free (lengths); free (lengths);
error (_("Out of memory allocating space for gnu histogram counts\n")); error (_("Out of memory allocating space for gnu histogram counts\n"));
return FALSE; goto err_out;
} }
for (hn = 0; hn < ngnubuckets; ++hn) for (hn = 0; hn < ngnubuckets; ++hn)
@ -12256,12 +12251,19 @@ process_symbol_table (Filedata * filedata)
free (counts); free (counts);
free (lengths); free (lengths);
free (gnubuckets);
free (gnuchains);
free (mipsxlat);
} }
free (gnubuckets);
free (gnuchains);
free (mipsxlat);
return TRUE; return TRUE;
err_out:
free (gnubuckets);
free (gnuchains);
free (mipsxlat);
free (buckets);
free (chains);
return FALSE;
} }
static bfd_boolean static bfd_boolean
@ -18773,6 +18775,14 @@ print_ia64_vms_note (Elf_Internal_Note * pnote)
return FALSE; return FALSE;
} }
struct build_attr_cache {
Filedata *filedata;
char *strtab;
unsigned long strtablen;
Elf_Internal_Sym *symtab;
unsigned long nsyms;
} ba_cache;
/* Find the symbol associated with a build attribute that is attached /* Find the symbol associated with a build attribute that is attached
to address OFFSET. If PNAME is non-NULL then store the name of to address OFFSET. If PNAME is non-NULL then store the name of
the symbol (if found) in the provided pointer, Returns NULL if a the symbol (if found) in the provided pointer, Returns NULL if a
@ -18784,19 +18794,19 @@ get_symbol_for_build_attribute (Filedata * filedata,
bfd_boolean is_open_attr, bfd_boolean is_open_attr,
const char ** pname) const char ** pname)
{ {
static Filedata * saved_filedata = NULL; Elf_Internal_Sym *saved_sym = NULL;
static char * strtab; Elf_Internal_Sym *sym;
static unsigned long strtablen;
static Elf_Internal_Sym * symtab;
static unsigned long nsyms;
Elf_Internal_Sym * saved_sym = NULL;
Elf_Internal_Sym * sym;
if (filedata->section_headers != NULL if (filedata->section_headers != NULL
&& (saved_filedata == NULL || filedata != saved_filedata)) && (ba_cache.filedata == NULL || filedata != ba_cache.filedata))
{ {
Elf_Internal_Shdr * symsec; Elf_Internal_Shdr * symsec;
free (ba_cache.strtab);
ba_cache.strtab = NULL;
free (ba_cache.symtab);
ba_cache.symtab = NULL;
/* Load the symbol and string sections. */ /* Load the symbol and string sections. */
for (symsec = filedata->section_headers; for (symsec = filedata->section_headers;
symsec < filedata->section_headers + filedata->file_header.e_shnum; symsec < filedata->section_headers + filedata->file_header.e_shnum;
@ -18804,41 +18814,52 @@ get_symbol_for_build_attribute (Filedata * filedata,
{ {
if (symsec->sh_type == SHT_SYMTAB) if (symsec->sh_type == SHT_SYMTAB)
{ {
symtab = GET_ELF_SYMBOLS (filedata, symsec, & nsyms); ba_cache.symtab = GET_ELF_SYMBOLS (filedata, symsec,
&ba_cache.nsyms);
if (symsec->sh_link < filedata->file_header.e_shnum) if (ba_cache.symtab != NULL
&& symsec->sh_link < filedata->file_header.e_shnum)
{ {
Elf_Internal_Shdr * strtab_sec = filedata->section_headers + symsec->sh_link; Elf_Internal_Shdr *strtab_sec
= filedata->section_headers + symsec->sh_link;
strtab = (char *) get_data (NULL, filedata, strtab_sec->sh_offset, ba_cache.strtab
1, strtab_sec->sh_size, = (char *) get_data (NULL, filedata, strtab_sec->sh_offset,
_("string table")); 1, strtab_sec->sh_size,
strtablen = strtab != NULL ? strtab_sec->sh_size : 0; _("string table"));
ba_cache.strtablen = strtab_sec->sh_size;
} }
if (ba_cache.strtab == NULL)
{
free (ba_cache.symtab);
ba_cache.symtab = NULL;
}
if (ba_cache.symtab != NULL)
break;
} }
} }
saved_filedata = filedata; ba_cache.filedata = filedata;
} }
if (symtab == NULL || strtab == NULL) if (ba_cache.symtab == NULL)
return NULL; return NULL;
/* Find a symbol whose value matches offset. */ /* Find a symbol whose value matches offset. */
for (sym = symtab; sym < symtab + nsyms; sym ++) for (sym = ba_cache.symtab; sym < ba_cache.symtab + ba_cache.nsyms; sym ++)
if (sym->st_value == offset) if (sym->st_value == offset)
{ {
if (sym->st_name >= strtablen) if (sym->st_name >= ba_cache.strtablen)
/* Huh ? This should not happen. */ /* Huh ? This should not happen. */
continue; continue;
if (strtab[sym->st_name] == 0) if (ba_cache.strtab[sym->st_name] == 0)
continue; continue;
/* The AArch64 and ARM architectures define mapping symbols /* The AArch64 and ARM architectures define mapping symbols
(eg $d, $x, $t) which we want to ignore. */ (eg $d, $x, $t) which we want to ignore. */
if (strtab[sym->st_name] == '$' if (ba_cache.strtab[sym->st_name] == '$'
&& strtab[sym->st_name + 1] != 0 && ba_cache.strtab[sym->st_name + 1] != 0
&& strtab[sym->st_name + 2] == 0) && ba_cache.strtab[sym->st_name + 2] == 0)
continue; continue;
if (is_open_attr) if (is_open_attr)
@ -18855,7 +18876,7 @@ get_symbol_for_build_attribute (Filedata * filedata,
{ {
/* If the symbol has a size associated /* If the symbol has a size associated
with it then we can stop searching. */ with it then we can stop searching. */
sym = symtab + nsyms; sym = ba_cache.symtab + ba_cache.nsyms;
} }
continue; continue;
@ -18895,7 +18916,7 @@ get_symbol_for_build_attribute (Filedata * filedata,
} }
if (saved_sym && pname) if (saved_sym && pname)
* pname = strtab + saved_sym->st_name; * pname = ba_cache.strtab + saved_sym->st_name;
return saved_sym; return saved_sym;
} }
@ -20238,6 +20259,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
putchar ('\n'); putchar ('\n');
free (qualified_name); free (qualified_name);
} }
free (member_name);
} }
} }
@ -20340,6 +20362,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
if (qualified_name == NULL) if (qualified_name == NULL)
{ {
error (_("%s: bad archive file name\n"), arch.file_name); error (_("%s: bad archive file name\n"), arch.file_name);
free (name);
ret = FALSE; ret = FALSE;
break; break;
} }
@ -20351,8 +20374,10 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
char * member_file_name = adjust_relative_path char * member_file_name = adjust_relative_path
(filedata->file_name, name, namelen); (filedata->file_name, name, namelen);
free (name);
if (member_file_name == NULL) if (member_file_name == NULL)
{ {
free (qualified_name);
ret = FALSE; ret = FALSE;
break; break;
} }
@ -20362,6 +20387,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
{ {
error (_("Input file '%s' is not readable.\n"), member_file_name); error (_("Input file '%s' is not readable.\n"), member_file_name);
free (member_file_name); free (member_file_name);
free (qualified_name);
ret = FALSE; ret = FALSE;
break; break;
} }
@ -20374,6 +20400,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
close_file (member_filedata); close_file (member_filedata);
free (member_file_name); free (member_file_name);
free (qualified_name);
} }
else if (is_thin_archive) else if (is_thin_archive)
{ {
@ -20386,9 +20413,12 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
{ {
error (_("%s: contains corrupt thin archive: %s\n"), error (_("%s: contains corrupt thin archive: %s\n"),
qualified_name, name); qualified_name, name);
free (qualified_name);
free (name);
ret = FALSE; ret = FALSE;
break; break;
} }
free (name);
/* This is a proxy for a member of a nested archive. */ /* This is a proxy for a member of a nested archive. */
archive_file_offset = arch.nested_member_origin + sizeof arch.arhdr; archive_file_offset = arch.nested_member_origin + sizeof arch.arhdr;
@ -20398,6 +20428,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
if (fseek (nested_arch.file, archive_file_offset, SEEK_SET) != 0) if (fseek (nested_arch.file, archive_file_offset, SEEK_SET) != 0)
{ {
error (_("%s: failed to seek to archive member.\n"), nested_arch.file_name); error (_("%s: failed to seek to archive member.\n"), nested_arch.file_name);
free (qualified_name);
ret = FALSE; ret = FALSE;
break; break;
} }
@ -20410,6 +20441,7 @@ process_archive (Filedata * filedata, bfd_boolean is_thin_archive)
} }
else else
{ {
free (name);
archive_file_offset = arch.next_arhdr_offset; archive_file_offset = arch.next_arhdr_offset;
arch.next_arhdr_offset += archive_file_size; arch.next_arhdr_offset += archive_file_size;
@ -20510,6 +20542,10 @@ process_file (char * file_name)
free (filedata->dump_sects); free (filedata->dump_sects);
free (filedata); free (filedata);
free (ba_cache.strtab);
free (ba_cache.symtab);
ba_cache.filedata = NULL;
return ret; return ret;
} }