From fbea15088db59186960134d11b8bf98070224d6c Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 11 Apr 2017 19:37:51 +0930 Subject: [PATCH] PR 21274, ld segfaults linking PE DLL Don't use fixed size buffers for symbol names. PR 21274 PR 18466 * emultempl/pe.em (pe_find_data_imports): Don't use fixed size symbol buffer. Instead, xmalloc max size needed with space for prefix. Wrap overlong lines. Formatting. Pass symbol buffer copy of name to pe_walk_relocs_of_symbol. (make_inport_fixup): Add "name" param, pass to pe_create_import_fixup. * emultempl/pe.em (pep_find_data_imports): As for pe_find_data_imports. (make_import_fixup): Add "name" param, pass to pep_create_import_fixup. Use bfd_get_signed_* and remove unnecessary casts. Formatting. * pe-dll.c (pe_walk_relocs_of_symbol): Add "name" param. Pass to callback. (make_import_fixup_mark): Add "name" param. Make use of prefix space rather than xmalloc here. (pe_create_import_fixup): Likewise. * pe-dll.h (pe_walk_relocs_of_symbol): Update prototype. (pe_create_import_fixup): Likewise. * pep-dll.h (pep_walk_relocs_of_symbol): Likewise. (pep_create_import_fixup): Likewise. --- ld/ChangeLog | 22 +++++++++ ld/emultempl/pe.em | 64 +++++++++++++++--------- ld/emultempl/pep.em | 116 +++++++++++++++++++++++++------------------- ld/pe-dll.c | 58 +++++++++------------- ld/pe-dll.h | 4 +- ld/pep-dll.h | 5 +- 6 files changed, 155 insertions(+), 114 deletions(-) diff --git a/ld/ChangeLog b/ld/ChangeLog index bc3120252d2..b55c436f703 100644 --- a/ld/ChangeLog +++ b/ld/ChangeLog @@ -1,3 +1,25 @@ +2017-04-11 Alan Modra + + PR 21274 + PR 18466 + * emultempl/pe.em (pe_find_data_imports): Don't use fixed size + symbol buffer. Instead, xmalloc max size needed with space for + prefix. Wrap overlong lines. Formatting. Pass symbol buffer + copy of name to pe_walk_relocs_of_symbol. + (make_inport_fixup): Add "name" param, pass to pe_create_import_fixup. + * emultempl/pe.em (pep_find_data_imports): As for pe_find_data_imports. + (make_import_fixup): Add "name" param, pass to pep_create_import_fixup. + Use bfd_get_signed_* and remove unnecessary casts. Formatting. + * pe-dll.c (pe_walk_relocs_of_symbol): Add "name" param. Pass to + callback. + (make_import_fixup_mark): Add "name" param. Make use of prefix + space rather than xmalloc here. + (pe_create_import_fixup): Likewise. + * pe-dll.h (pe_walk_relocs_of_symbol): Update prototype. + (pe_create_import_fixup): Likewise. + * pep-dll.h (pep_walk_relocs_of_symbol): Likewise. + (pep_create_import_fixup): Likewise. + 2017-04-10 Nick Clifton * ld.texinfo (--strip-discarded): Document. diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em index 8f41f27709a..e835404479f 100644 --- a/ld/emultempl/pe.em +++ b/ld/emultempl/pe.em @@ -1130,7 +1130,7 @@ pe_fixup_stdcalls (void) } static int -make_import_fixup (arelent *rel, asection *s) +make_import_fixup (arelent *rel, asection *s, char *name) { struct bfd_symbol *sym = *rel->sym_ptr_ptr; char addend[4]; @@ -1143,7 +1143,7 @@ make_import_fixup (arelent *rel, asection *s) einfo (_("%C: Cannot get section contents - auto-import exception\n"), s->owner, s, rel->address); - pe_create_import_fixup (rel, s, bfd_get_32 (s->owner, addend)); + pe_create_import_fixup (rel, s, bfd_get_32 (s->owner, addend), name); return 1; } @@ -1152,32 +1152,46 @@ static void pe_find_data_imports (void) { struct bfd_link_hash_entry *undef, *sym; + size_t namelen; + char *buf, *name; if (link_info.pei386_auto_import == 0) return; - for (undef = link_info.hash->undefs; undef; undef=undef->u.undef.next) + namelen = 0; + for (undef = link_info.hash->undefs; undef; undef = undef->u.undef.next) + { + if (undef->type == bfd_link_hash_undefined) + { + size_t len = strlen (undef->root.string); + if (namelen < len) + namelen = len; + } + } + if (namelen == 0) + return; + + /* We are being a bit cunning here. The buffer will have space for + prefixes at the beginning. The prefix is modified here and in a + number of functions called from this function. */ +#define PREFIX_LEN 32 + buf = xmalloc (PREFIX_LEN + namelen + 1); + name = buf + PREFIX_LEN; + + for (undef = link_info.hash->undefs; undef; undef = undef->u.undef.next) { if (undef->type == bfd_link_hash_undefined) { - /* C++ symbols are *long*. */ -#define BUF_SIZE 4096 - char buf[BUF_SIZE]; + char *impname; if (pe_dll_extra_pe_debug) printf ("%s:%s\n", __FUNCTION__, undef->root.string); - if (strlen (undef->root.string) > (BUF_SIZE - 6)) - { - /* PR linker/18466. */ - einfo (_("%P: internal error: symbol too long: %s\n"), - undef->root.string); - return; - } - - sprintf (buf, "__imp_%s", undef->root.string); + strcpy (name, undef->root.string); + impname = name - (sizeof "__imp_" - 1); + memcpy (impname, "__imp_", sizeof "__imp_" - 1); - sym = bfd_link_hash_lookup (link_info.hash, buf, 0, 0, 1); + sym = bfd_link_hash_lookup (link_info.hash, impname, 0, 0, 1); if (sym && sym->type == bfd_link_hash_defined) { @@ -1189,15 +1203,18 @@ pe_find_data_imports (void) { static bfd_boolean warned = FALSE; - info_msg (_("Info: resolving %s by linking to %s (auto-import)\n"), - undef->root.string, buf); + info_msg (_("Info: resolving %s by linking to %s " + "(auto-import)\n"), name, impname); /* PR linker/4844. */ if (! warned) { warned = TRUE; - einfo (_("%P: warning: auto-importing has been activated without --enable-auto-import specified on the command line.\n\ -This should work unless it involves constant data structures referencing symbols from auto-imported DLLs.\n")); + einfo (_("%P: warning: auto-importing has been activated " + "without --enable-auto-import specified on the " + "command line.\nThis should work unless it " + "involves constant data structures referencing " + "symbols from auto-imported DLLs.\n")); } } @@ -1212,8 +1229,7 @@ This should work unless it involves constant data structures referencing symbols for (i = 0; i < nsyms; i++) { - if (! CONST_STRNEQ (symbols[i]->name, - U ("_head_"))) + if (! CONST_STRNEQ (symbols[i]->name, U ("_head_"))) continue; if (pe_dll_extra_pe_debug) @@ -1224,8 +1240,7 @@ This should work unless it involves constant data structures referencing symbols break; } - pe_walk_relocs_of_symbol (&link_info, undef->root.string, - make_import_fixup); + pe_walk_relocs_of_symbol (&link_info, name, make_import_fixup); /* Let's differentiate it somehow from defined. */ undef->type = bfd_link_hash_defweak; @@ -1238,6 +1253,7 @@ This should work unless it involves constant data structures referencing symbols } } } + free (buf); } static bfd_boolean diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em index 8c7ed88c361..59812ce3d8f 100644 --- a/ld/emultempl/pep.em +++ b/ld/emultempl/pep.em @@ -1075,7 +1075,7 @@ pep_fixup_stdcalls (void) } static int -make_import_fixup (arelent *rel, asection *s) +make_import_fixup (arelent *rel, asection *s, char *name) { struct bfd_symbol *sym = *rel->sym_ptr_ptr; char addend[8]; @@ -1089,32 +1089,32 @@ make_import_fixup (arelent *rel, asection *s) memset (addend, 0, sizeof (addend)); switch ((rel->howto->bitsize)) { - case 8: - suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 1); - if (suc && rel->howto->pc_relative) - _addend = (bfd_vma) ((bfd_signed_vma) ((char) bfd_get_8 (s->owner, addend))); - else if (suc) - _addend = ((bfd_vma) bfd_get_8 (s->owner, addend)) & 0xff; - break; - case 16: - suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 2); - if (suc && rel->howto->pc_relative) - _addend = (bfd_vma) ((bfd_signed_vma) ((short) bfd_get_16 (s->owner, addend))); - else if (suc) - _addend = ((bfd_vma) bfd_get_16 (s->owner, addend)) & 0xffff; - break; - case 32: - suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 4); - if (suc && rel->howto->pc_relative) - _addend = (bfd_vma) ((bfd_signed_vma) ((int) bfd_get_32 (s->owner, addend))); - else if (suc) - _addend = ((bfd_vma) bfd_get_32 (s->owner, addend)) & 0xffffffff; - break; - case 64: - suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 8); - if (suc) - _addend = ((bfd_vma) bfd_get_64 (s->owner, addend)); - break; + case 8: + suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 1); + if (suc && rel->howto->pc_relative) + _addend = bfd_get_signed_8 (s->owner, addend); + else if (suc) + _addend = bfd_get_8 (s->owner, addend); + break; + case 16: + suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 2); + if (suc && rel->howto->pc_relative) + _addend = bfd_get_signed_16 (s->owner, addend); + else if (suc) + _addend = bfd_get_16 (s->owner, addend); + break; + case 32: + suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 4); + if (suc && rel->howto->pc_relative) + _addend = bfd_get_signed_32 (s->owner, addend); + else if (suc) + _addend = bfd_get_32 (s->owner, addend); + break; + case 64: + suc = bfd_get_section_contents (s->owner, s, addend, rel->address, 8); + if (suc) + _addend = bfd_get_64 (s->owner, addend); + break; } if (! suc) einfo (_("%C: Cannot get section contents - auto-import exception\n"), @@ -1122,11 +1122,13 @@ make_import_fixup (arelent *rel, asection *s) if (pep_dll_extra_pe_debug) { - printf ("import of 0x%lx(0x%lx) sec_addr=0x%lx", (long) _addend, (long) rel->addend, (long) rel->address); - if (rel->howto->pc_relative) printf (" pcrel"); - printf (" %d bit rel.\n",(int) rel->howto->bitsize); - } - pep_create_import_fixup (rel, s, _addend); + printf ("import of 0x%lx(0x%lx) sec_addr=0x%lx", + (long) _addend, (long) rel->addend, (long) rel->address); + if (rel->howto->pc_relative) + printf (" pcrel"); + printf (" %d bit rel.\n", (int) rel->howto->bitsize); + } + pep_create_import_fixup (rel, s, _addend, name); return 1; } @@ -1135,32 +1137,46 @@ static void pep_find_data_imports (void) { struct bfd_link_hash_entry *undef, *sym; + size_t namelen; + char *buf, *name; if (link_info.pei386_auto_import == 0) return; - for (undef = link_info.hash->undefs; undef; undef=undef->u.undef.next) + namelen = 0; + for (undef = link_info.hash->undefs; undef; undef = undef->u.undef.next) { if (undef->type == bfd_link_hash_undefined) { - /* C++ symbols are *long*. */ -#define BUF_SIZE 4096 - char buf[BUF_SIZE]; + size_t len = strlen (undef->root.string); + if (namelen < len) + namelen = len; + } + } + if (namelen == 0) + return; + + /* We are being a bit cunning here. The buffer will have space for + prefixes at the beginning. The prefix is modified here and in a + number of functions called from this function. */ +#define PREFIX_LEN 32 + buf = xmalloc (PREFIX_LEN + namelen + 1); + name = buf + PREFIX_LEN; + + for (undef = link_info.hash->undefs; undef; undef = undef->u.undef.next) + { + if (undef->type == bfd_link_hash_undefined) + { + char *impname; if (pep_dll_extra_pe_debug) printf ("%s:%s\n", __FUNCTION__, undef->root.string); - if (strlen (undef->root.string) > (BUF_SIZE - 6)) - { - /* PR linker/18466. */ - einfo (_("%P: internal error: symbol too long: %s\n"), - undef->root.string); - return; - } - - sprintf (buf, "__imp_%s", undef->root.string); + strcpy (name, undef->root.string); + impname = name - (sizeof "__imp_" - 1); + memcpy (impname, "__imp_", sizeof "__imp_" - 1); - sym = bfd_link_hash_lookup (link_info.hash, buf, 0, 0, 1); + sym = bfd_link_hash_lookup (link_info.hash, impname, 0, 0, 1); if (sym && sym->type == bfd_link_hash_defined) { @@ -1185,13 +1201,12 @@ pep_find_data_imports (void) if (pep_dll_extra_pe_debug) printf ("->%s\n", symbols[i]->name); - pep_data_import_dll = (char*) (symbols[i]->name + - U_SIZE ("_head_") - 1); + pep_data_import_dll = (char *) (symbols[i]->name + + U_SIZE ("_head_") - 1); break; } - pep_walk_relocs_of_symbol (&link_info, undef->root.string, - make_import_fixup); + pep_walk_relocs_of_symbol (&link_info, name, make_import_fixup); /* Let's differentiate it somehow from defined. */ undef->type = bfd_link_hash_defweak; @@ -1204,6 +1219,7 @@ pep_find_data_imports (void) } } } + free (buf); } static bfd_boolean diff --git a/ld/pe-dll.c b/ld/pe-dll.c index bb17b7dca6b..4ce7a3638cb 100644 --- a/ld/pe-dll.c +++ b/ld/pe-dll.c @@ -1275,8 +1275,8 @@ static struct bfd_section *current_sec; void pe_walk_relocs_of_symbol (struct bfd_link_info *info, - const char *name, - int (*cb) (arelent *, asection *)) + char *name, + int (*cb) (arelent *, asection *, char *)) { bfd *b; asection *s; @@ -1315,7 +1315,7 @@ pe_walk_relocs_of_symbol (struct bfd_link_info *info, struct bfd_symbol *sym = *relocs[i]->sym_ptr_ptr; if (!strcmp (name, sym->name)) - cb (relocs[i], s); + cb (relocs[i], s, name); } free (relocs); @@ -2396,37 +2396,21 @@ make_singleton_name_thunk (const char *import, bfd *parent) } static char * -make_import_fixup_mark (arelent *rel) +make_import_fixup_mark (arelent *rel, char *name) { /* We convert reloc to symbol, for later reference. */ - static int counter; - static char *fixup_name = NULL; - static size_t buffer_len = 0; - + static unsigned int counter; struct bfd_symbol *sym = *rel->sym_ptr_ptr; - bfd *abfd = bfd_asymbol_bfd (sym); struct bfd_link_hash_entry *bh; + char *fixup_name, buf[26]; + size_t prefix_len; - if (!fixup_name) - { - fixup_name = xmalloc (384); - buffer_len = 384; - } - - if (strlen (sym->name) + 25 > buffer_len) - /* Assume 25 chars for "__fu" + counter + "_". If counter is - bigger than 20 digits long, we've got worse problems than - overflowing this buffer... */ - { - free (fixup_name); - /* New buffer size is length of symbol, plus 25, but - then rounded up to the nearest multiple of 128. */ - buffer_len = ((strlen (sym->name) + 25) + 127) & ~127; - fixup_name = xmalloc (buffer_len); - } - - sprintf (fixup_name, "__fu%d_%s", counter++, sym->name); + /* "name" buffer has space before the symbol name for prefixes. */ + sprintf (buf, "__fu%d_", counter++); + prefix_len = strlen (buf); + fixup_name = name - prefix_len; + memcpy (fixup_name, buf, prefix_len); bh = NULL; bfd_coff_link_add_one_symbol (&link_info, abfd, fixup_name, BSF_GLOBAL, @@ -2626,23 +2610,25 @@ pe_create_runtime_relocator_reference (bfd *parent) } void -pe_create_import_fixup (arelent *rel, asection *s, bfd_vma addend) +pe_create_import_fixup (arelent *rel, asection *s, bfd_vma addend, char *name) { - char buf[300]; struct bfd_symbol *sym = *rel->sym_ptr_ptr; struct bfd_link_hash_entry *name_thunk_sym; struct bfd_link_hash_entry *name_imp_sym; - const char *name = sym->name; - char *fixup_name = make_import_fixup_mark (rel); + char *fixup_name, *impname; bfd *b; int need_import_table = 1; - sprintf (buf, "__imp_%s", name); - name_imp_sym = bfd_link_hash_lookup (link_info.hash, buf, 0, 0, 1); + /* name buffer is allocated with space at beginning for prefixes. */ + impname = name - (sizeof "__imp_" - 1); + memcpy (impname, "__imp_", sizeof "__imp_" - 1); + name_imp_sym = bfd_link_hash_lookup (link_info.hash, impname, 0, 0, 1); - sprintf (buf, "__nm_thnk_%s", name); + impname = name - (sizeof "__nm_thnk_" - 1); + memcpy (impname, "__nm_thnk_", sizeof "__nm_thnk_" - 1); + name_thunk_sym = bfd_link_hash_lookup (link_info.hash, impname, 0, 0, 1); - name_thunk_sym = bfd_link_hash_lookup (link_info.hash, buf, 0, 0, 1); + fixup_name = make_import_fixup_mark (rel, name); /* For version 2 pseudo relocation we don't need to add an import if the import symbol is already present. */ diff --git a/ld/pe-dll.h b/ld/pe-dll.h index 6d501252e06..de809243341 100644 --- a/ld/pe-dll.h +++ b/ld/pe-dll.h @@ -62,9 +62,9 @@ extern void pe_dll_fill_sections extern void pe_exe_fill_sections (bfd *, struct bfd_link_info *); extern void pe_walk_relocs_of_symbol - (struct bfd_link_info *, const char *, int (*) (arelent *, asection *)); + (struct bfd_link_info *, char *, int (*) (arelent *, asection *, char *)); extern void pe_create_import_fixup - (arelent * rel, asection *, bfd_vma); + (arelent * rel, asection *, bfd_vma, char *); extern bfd_boolean pe_bfd_is_dll (bfd *); extern void pe_output_file_set_long_section_names diff --git a/ld/pep-dll.h b/ld/pep-dll.h index 8a4baab28fe..07b4919d10e 100644 --- a/ld/pep-dll.h +++ b/ld/pep-dll.h @@ -53,8 +53,9 @@ extern void pep_exe_build_sections (bfd *, struct bfd_link_info *); extern void pep_dll_fill_sections (bfd *, struct bfd_link_info *); extern void pep_exe_fill_sections (bfd *, struct bfd_link_info *); extern void pep_walk_relocs_of_symbol - (struct bfd_link_info *, const char *, int (*) (arelent *, asection *)); -extern void pep_create_import_fixup (arelent * rel, asection *, bfd_vma); + (struct bfd_link_info *, char *, int (*) (arelent *, asection *, char *)); +extern void pep_create_import_fixup (arelent * rel, asection *, bfd_vma, + char *); extern bfd_boolean pep_bfd_is_dll (bfd *); extern void pep_output_file_set_long_section_names (bfd *); -- 2.30.2