From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x435.google.com (mail-pf1-x435.google.com [IPv6:2607:f8b0:4864:20::435]) by sourceware.org (Postfix) with ESMTPS id 9E3663857427 for ; Wed, 11 May 2022 12:49:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9E3663857427 Received: by mail-pf1-x435.google.com with SMTP id bo5so1909589pfb.4 for ; Wed, 11 May 2022 05:49:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=gFvUGiRIUWwVPMcLdGvjOMs4xFf8EvVkgZnfb9F51zc=; b=nx5jZ3jeFtcWWBGll8hTQv+Lki9YqvEMrU4BkHsMoPXwMok7iCV75OkHHyalO0u0GW vMWYuDuLqNCRoYOJpNqKn2yJF/L3/fuiA1ub2r/zXOZVD9b53whBC1DjNVJwzkCfWuzI m6/4vnJt5fuIMZgTc08av4LFRdgmemJ4YNPTmlo67lomldLoeODdMYw+iKlJp+fGLWyT cF/dsC/ugd+W6zHplaAg7AiF54RmGrEH503h1LvOs8ecTeXcAj1IaVMO6iH6o3PGvxM/ U66zjCz26a+69Dbg18KuHZZPh2/zBfgMqx5A7bAkhgjR0Q9xgK2mBiHmUfSU5/5RetiR CzKw== X-Gm-Message-State: AOAM531nQcJ2KV2h/zLSrrwyouovxiFL+2Z5/xbDVc/K4FZ6ydCMb6m9 3VDn3UsmB2zWIM1FNXgh6+LrtSPYRZQ= X-Google-Smtp-Source: ABdhPJzE08TMMuJK65/1Rux2OWgALQ96r2iwQebBSAlOYLj3OQnlT++cUX1fV5hq6Q0J9e3CRFTl7g== X-Received: by 2002:a63:da13:0:b0:3c6:4c0:e2f9 with SMTP id c19-20020a63da13000000b003c604c0e2f9mr20698858pgh.493.1652273377557; Wed, 11 May 2022 05:49:37 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id a127-20020a621a85000000b0050dc7628168sm1664172pfa.66.2022.05.11.05.49.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 May 2022 05:49:36 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 6D88B1140AD4; Wed, 11 May 2022 22:19:33 +0930 (ACST) Date: Wed, 11 May 2022 22:19:33 +0930 From: Alan Modra To: Roland Schwingel Cc: Binutils Subject: Re: LD: Invalid memory access when linking a dll (maybe Bug 29006 revisited) Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3037.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 May 2022 12:49:41 -0000 On Tue, May 10, 2022 at 07:19:03PM +0200, Roland Schwingel wrote: > ==23381== Invalid read of size 1 > ==23381== at 0x508B434: vfprintf (in /lib64/libc-2.17.so) > ==23381== by 0x50B3E63: vasprintf (in /lib64/libc-2.17.so) > ==23381== by 0x50912F6: asprintf (in /lib64/libc-2.17.so) > ==23381== by 0x4432CE: make_runtime_pseudo_reloc (pe-dll.c:2663) > ==23381== by 0x443A81: pep_create_import_fixup (pe-dll.c:2838) > ==23381== by 0x432CA6: make_import_fixup (ei386pep.c:1129) > ==23381== by 0x43F8A5: pe_walk_relocs (pe-dll.c:1349) > ==23381== by 0x43FD95: pep_find_data_imports (pe-dll.c:1497) > ==23381== by 0x433674: gld_i386pep_after_open (ei386pep.c:1408) OK, so we are running pep_find_data_imports and happen to call make_runtime_pseudo_reloc while inpecting relocs. make_runtime_pseudo_reloc uses dll_symname, which has been cleared > ==23381== by 0x428FCB: ldemul_after_open (ldemul.c:65) > ==23381== by 0x41D9F2: lang_process (ldlang.c:8162) > ==23381== by 0x422440: main (ldmain.c:497) > ==23381== Address 0x95e2500 is 0 bytes inside a block of size 20 free'd > ==23381== at 0x4C2E10B: free (vg_replace_malloc.c:871) > ==23381== by 0x445199: pep_process_import_defs (pe-dll.c:3324) after finishing pep_process_import_defs. These two functions are called in close proximity. See emultempl/pep.em. My guess is that this broke with commit c4a8df19ba0a. > ==23381== by 0x433648: gld_i386pep_after_open (ei386pep.c:1405) > ==23381== by 0x428FCB: ldemul_after_open (ldemul.c:65) > ==23381== by 0x41D9F2: lang_process (ldlang.c:8162) > ==23381== by 0x422440: main (ldmain.c:497) Looks like we need to set up dll_symname earlier now. Please try this patch. PR 29006 * pe-dll.c (dll_name): Delete, replacing with.. (dll_filename): ..this, moved earlier in file. (generate_edata): Delete parameters. Don't set up dll_name here.. (pe_process_import_defs): ..instead set up dll_filename and dll_symname here before returning. (dll_symname_len): Delete write-only variable. (pe_dll_generate_implib): Don't set up dll_symname here. diff --git a/ld/pe-dll.c b/ld/pe-dll.c index ed68f66a95b..4cf8ed23672 100644 --- a/ld/pe-dll.c +++ b/ld/pe-dll.c @@ -505,7 +505,7 @@ static int export_table_size; static int count_exported; static int count_exported_byname; static int count_with_ordinals; -static const char *dll_name; +static const char *dll_filename; static int min_ordinal, max_ordinal; static int *exported_symbols; @@ -1066,25 +1066,13 @@ build_filler_bfd (int include_edata) /* Gather all the exported symbols and build the .edata section. */ static void -generate_edata (bfd *abfd, struct bfd_link_info *info ATTRIBUTE_UNUSED) +generate_edata (void) { int i, next_ordinal; int name_table_size = 0; - const char *dlnp; /* First, we need to know how many exported symbols there are, and what the range of ordinals is. */ - if (pe_def_file->name) - dll_name = pe_def_file->name; - else - { - dll_name = bfd_get_filename (abfd); - - for (dlnp = dll_name; *dlnp; dlnp++) - if (*dlnp == '\\' || *dlnp == '/' || *dlnp == ':') - dll_name = dlnp + 1; - } - if (count_with_ordinals && max_ordinal > count_exported) { if (min_ordinal > max_ordinal - count_exported + 1) @@ -1159,7 +1147,7 @@ generate_edata (bfd *abfd, struct bfd_link_info *info ATTRIBUTE_UNUSED) + 4 * export_table_size /* addresses */ + 4 * count_exported_byname /* name ptrs */ + 2 * count_exported_byname /* ordinals */ - + name_table_size + strlen (dll_name) + 1); + + name_table_size + strlen (dll_filename) + 1); } /* Fill the exported symbol offsets. The preliminary work has already @@ -1232,7 +1220,7 @@ fill_edata (bfd *abfd, struct bfd_link_info *info ATTRIBUTE_UNUSED) } bfd_put_32 (abfd, ERVA (enamestr), edata_d + 12); - strcpy (enamestr, dll_name); + strcpy (enamestr, dll_filename); enamestr += strlen (enamestr) + 1; bfd_put_32 (abfd, min_ordinal, edata_d + 16); bfd_put_32 (abfd, export_table_size, edata_d + 20); @@ -1971,9 +1959,7 @@ pe_dll_generate_def_file (const char *pe_out_def_filename) static asymbol **symtab; static int symptr; static int tmp_seq; -static const char *dll_filename; static char *dll_symname; -static int dll_symname_len; #define UNDSEC bfd_und_section_ptr @@ -2835,7 +2821,8 @@ pe_create_import_fixup (arelent *rel, asection *s, bfd_vma addend, char *name, printf ("creating runtime pseudo-reloc entry for %s (addend=%d)\n", fixup_name, (int) addend); - b = make_runtime_pseudo_reloc (name, fixup_name, addend, rel->howto->bitsize, + b = make_runtime_pseudo_reloc (name, fixup_name, addend, + rel->howto->bitsize, link_info.output_bfd); add_bfd_to_link (b, bfd_get_filename (b), &link_info); @@ -2861,13 +2848,6 @@ pe_dll_generate_implib (def_file *def, const char *impfilename, struct bfd_link_ bfd *ibfd; bfd *head = 0; - dll_filename = (def->name) ? def->name : dll_name; - dll_symname = xstrdup (dll_filename); - dll_symname_len = strlen (dll_symname); - for (i = 0; dll_symname[i]; i++) - if (!ISALNUM (dll_symname[i])) - dll_symname[i] = '_'; - unlink_if_ordinary (impfilename); outarch = bfd_openw (impfilename, 0); @@ -2995,8 +2975,7 @@ pe_dll_generate_implib (def_file *def, const char *impfilename, struct bfd_link_ } } - n = make_one (def->exports + i, outarch, - ! (def->exports + i)->flag_data); + n = make_one (def->exports + i, outarch, !(def->exports + i)->flag_data); n->archive_next = head; head = n; def->exports[i].internal_name = internal; @@ -3208,128 +3187,145 @@ add_bfd_to_link (bfd *abfd, const char *name, struct bfd_link_info *linfo) void pe_process_import_defs (bfd *output_bfd, struct bfd_link_info *linfo) { - int i, j; - def_file_module *module; - def_file_import *imp; - pe_dll_id_target (bfd_get_target (output_bfd)); - if (!pe_def_file) - return; - - imp = pe_def_file->imports; - - pe_create_undef_table (); - - for (module = pe_def_file->modules; module; module = module->next) + if (pe_def_file) { - int do_this_dll = 0; + int i, j; + def_file_module *module; + def_file_import *imp; - for (i = 0; i < pe_def_file->num_imports && imp[i].module != module; i++) - ; - if (i >= pe_def_file->num_imports) - continue; + imp = pe_def_file->imports; - dll_filename = module->name; - dll_symname = xstrdup (module->name); - dll_symname_len = strlen (dll_symname); - for (j = 0; dll_symname[j]; j++) - if (!ISALNUM (dll_symname[j])) - dll_symname[j] = '_'; + pe_create_undef_table (); - for (; i < pe_def_file->num_imports && imp[i].module == module; i++) + for (module = pe_def_file->modules; module; module = module->next) { - def_file_export exp; - struct bfd_link_hash_entry *blhe; - int lead_at = (*imp[i].internal_name == '@'); - /* See if we need this import. */ - size_t len = strlen (imp[i].internal_name); - char *name = xmalloc (len + 2 + 6); - bool include_jmp_stub = false; - bool is_cdecl = false; - bool is_undef = false; - - if (!lead_at && strchr (imp[i].internal_name, '@') == NULL) - is_cdecl = true; - - if (lead_at) - sprintf (name, "%s", imp[i].internal_name); - else - sprintf (name, "%s%s",U (""), imp[i].internal_name); + int do_this_dll = 0; - blhe = bfd_link_hash_lookup (linfo->hash, name, - false, false, false); + for (i = 0; i < pe_def_file->num_imports; i++) + if (imp[i].module == module) + break; + if (i >= pe_def_file->num_imports) + continue; + + dll_filename = module->name; + dll_symname = xstrdup (module->name); + for (j = 0; dll_symname[j]; j++) + if (!ISALNUM (dll_symname[j])) + dll_symname[j] = '_'; - /* Include the jump stub for only if the - is undefined. */ - if (!blhe || (blhe && blhe->type != bfd_link_hash_undefined)) + for (; i < pe_def_file->num_imports && imp[i].module == module; i++) { + def_file_export exp; + struct bfd_link_hash_entry *blhe; + int lead_at = (*imp[i].internal_name == '@'); + /* See if we need this import. */ + size_t len = strlen (imp[i].internal_name); + char *name = xmalloc (len + 2 + 6); + bool include_jmp_stub = false; + bool is_cdecl = false; + bool is_undef = false; + + if (!lead_at && strchr (imp[i].internal_name, '@') == NULL) + is_cdecl = true; + if (lead_at) - sprintf (name, "%s%s", "__imp_", imp[i].internal_name); + sprintf (name, "%s", imp[i].internal_name); else - sprintf (name, "%s%s%s", "__imp_", U (""), - imp[i].internal_name); + sprintf (name, "%s%s",U (""), imp[i].internal_name); blhe = bfd_link_hash_lookup (linfo->hash, name, false, false, false); - if (blhe) - is_undef = (blhe->type == bfd_link_hash_undefined); - } - else - { - include_jmp_stub = true; - is_undef = (blhe->type == bfd_link_hash_undefined); - } - if (is_cdecl && (!blhe || (blhe && blhe->type != bfd_link_hash_undefined))) - { - sprintf (name, "%s%s",U (""), imp[i].internal_name); - blhe = pe_find_cdecl_alias_match (linfo, name); - include_jmp_stub = true; - if (blhe) - is_undef = (blhe->type == bfd_link_hash_undefined); - } + /* Include the jump stub for only if the + is undefined. */ + if (!blhe || (blhe && blhe->type != bfd_link_hash_undefined)) + { + if (lead_at) + sprintf (name, "%s%s", "__imp_", imp[i].internal_name); + else + sprintf (name, "%s%s%s", "__imp_", U (""), + imp[i].internal_name); + + blhe = bfd_link_hash_lookup (linfo->hash, name, + false, false, false); + if (blhe) + is_undef = (blhe->type == bfd_link_hash_undefined); + } + else + { + include_jmp_stub = true; + is_undef = (blhe->type == bfd_link_hash_undefined); + } + + if (is_cdecl + && (!blhe || (blhe && blhe->type != bfd_link_hash_undefined))) + { + sprintf (name, "%s%s",U (""), imp[i].internal_name); + blhe = pe_find_cdecl_alias_match (linfo, name); + include_jmp_stub = true; + if (blhe) + is_undef = (blhe->type == bfd_link_hash_undefined); + } - free (name); + free (name); - if (is_undef) - { - bfd *one; - /* We do. */ - if (!do_this_dll) + if (is_undef) { - bfd *ar_head = make_head (output_bfd); - add_bfd_to_link (ar_head, bfd_get_filename (ar_head), linfo); - do_this_dll = 1; + bfd *one; + /* We do. */ + if (!do_this_dll) + { + bfd *ar_head = make_head (output_bfd); + add_bfd_to_link (ar_head, bfd_get_filename (ar_head), + linfo); + do_this_dll = 1; + } + exp.internal_name = imp[i].internal_name; + exp.name = imp[i].name; + exp.its_name = imp[i].its_name; + exp.ordinal = imp[i].ordinal; + exp.hint = exp.ordinal >= 0 ? exp.ordinal : 0; + exp.flag_private = 0; + exp.flag_constant = 0; + exp.flag_data = imp[i].data; + exp.flag_noname = exp.name ? 0 : 1; + one = make_one (&exp, output_bfd, + !exp.flag_data && include_jmp_stub); + add_bfd_to_link (one, bfd_get_filename (one), linfo); } - exp.internal_name = imp[i].internal_name; - exp.name = imp[i].name; - exp.its_name = imp[i].its_name; - exp.ordinal = imp[i].ordinal; - exp.hint = exp.ordinal >= 0 ? exp.ordinal : 0; - exp.flag_private = 0; - exp.flag_constant = 0; - exp.flag_data = imp[i].data; - exp.flag_noname = exp.name ? 0 : 1; - one = make_one (&exp, output_bfd, (! exp.flag_data) && include_jmp_stub); - add_bfd_to_link (one, bfd_get_filename (one), linfo); } + if (do_this_dll) + { + bfd *ar_tail = make_tail (output_bfd); + add_bfd_to_link (ar_tail, bfd_get_filename (ar_tail), linfo); + } + + free (dll_symname); } - if (do_this_dll) + + while (undef_count) { - bfd *ar_tail = make_tail (output_bfd); - add_bfd_to_link (ar_tail, bfd_get_filename (ar_tail), linfo); + --undef_count; + free (udef_table[undef_count].key); } - - free (dll_symname); + free (udef_table); } - while (undef_count) + if (pe_def_file && pe_def_file->name) + dll_filename = pe_def_file->name; + else { - --undef_count; - free (udef_table[undef_count].key); + dll_filename = bfd_get_filename (output_bfd); + for (const char *p = dll_filename; *p; p++) + if (*p == '\\' || *p == '/' || *p == ':') + dll_filename = p + 1; } - free (udef_table); + dll_symname = xstrdup (dll_filename); + for (int i = 0; dll_symname[i]; i++) + if (!ISALNUM (dll_symname[i])) + dll_symname[i] = '_'; } /* We were handed a *.DLL file. Parse it and turn it into a set of @@ -3629,7 +3625,7 @@ pe_dll_build_sections (bfd *abfd, struct bfd_link_info *info) return; } - generate_edata (abfd, info); + generate_edata (); build_filler_bfd (1); pe_output_file_set_long_section_names (filler_bfd); } -- Alan Modra Australia Development Lab, IBM