This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH]: Fix for broken PE auto-importing.
- From: Pedro Alves <pedro_alves at portugalmail dot pt>
- To: binutils at sourceware dot org
- Date: Sun, 29 Oct 2006 16:01:27 +0000
- Subject: [PATCH]: Fix for broken PE auto-importing.
Hi all,
Refering to the thread at:
http://sourceware.org/ml/binutils/2006-09/msg00119.html
Kai Tietz wrote:
> Hallo,
>
> I found a memory violation in the function
"make_singleton_name_thunk" of
> pe_dll.c file. There is allocated a heap buffer of 4 bytes and
afterwards
> memset this pointer with length of 8 bytes,
>
> --- src/ld/pe-dll.c 2006-08-21 10:12:46.000000000 +0200
> +++ src_n/ld/pe-dll.c 2006-09-15 12:07:39.000000000 +0200
> @@ -2036,7 +2036,7 @@
> quick_symbol (abfd, U ("_nm_"), import, "", UNDSEC, BSF_GLOBAL, 0);
>
> bfd_set_section_size (abfd, id4, 8);
> - d4 = xmalloc (4);
> + d4 = xmalloc (8);
> id4->contents = d4;
> memset (d4, 0, 8);
> quick_reloc (abfd, 0, BFD_RELOC_RVA, 2);
>
>
> Regards,
> i.A. Kai Tietz
>
>
> PS: This piece of code brought me to the question, why this thunk
gets an
> empty one plus ?
>
>
>
Its was a required NULL terminator.
Kai Tietz also wrote:
> Hallo Dave,
>
> I meant by an empty one the fact, that the idata4 element (called thumb)
> has under i(3456)86 PE a size of 4 bytes. Therefore, if there are
stored 8
> bytes - as it is - it means there is an additional empty (zero) thumb
> present.
>
Right, but it is required.
> In the patch for PE+ for x86_64 I modified this code to the size of one
> thumb element and I didn't found any problems for PE. In my pending
patch,
> I replaced the IDATA4 and IDATA5 sizes by constances, because it is ugly
> to read.
>
> Regards,
> i.A. Kai Tietz
>
> PS: Under the x86_64 target one thumb element gets really the size of 8
> bytes.
>
>
>
But it was removed with the pe+ support patch (PE_IDATA4_SIZE == 4 in
32bit archs):
- bfd_set_section_size (abfd, id4, 8);
- d4 = xmalloc (4);
+ bfd_set_section_size (abfd, id4, PE_IDATA4_SIZE);
+ d4 = xmalloc (PE_IDATA4_SIZE);
id4->contents = d4;
- memset (d4, 0, 8);
+ memset (d4, 0, PE_IDATA4_SIZE);
quick_reloc (abfd, 0, BFD_RELOC_RVA, 2);
save_relocs (id4);
bfd_set_symtab (abfd, symtab, symptr);
- bfd_set_section_contents (abfd, id4, d4, 0, 8);
+ bfd_set_section_contents (abfd, id4, d4, 0, PE_IDATA4_SIZE);
-----
Refer to this comment from dlltool.c:
.idata$4 = Import Lookup Table
= array of array of pointers to hint name table.
There is one for each dll being imported from, and each dll's set is
terminated by a trailing NULL.
The extra 4 bytes where that trailing NULL.
The effect of removing it is something like this:
This is the dump of the import table for the auto-import thunk. It
should only contain one entry, the auto import of appUniqueID ...
DLL Name: QtCored4.dll
vma: Hint/Ord Member-Name Bound-To
5d549c 2701 appUniqueID
5d54cc 2705 qt_cever
80000300 768 <none>
80000575 1397 <none>
80000417 1047 <none>
800002ee 750 <none>
(snip a couple hundred broken imports.)
... like this, with attached patch applied:
DLL Name: QtCored4.dll
vma: Hint/Ord Member-Name Bound-To
5d54a4 2701 appUniqueID
--
This can only be triggered in some combinations of mixing MSFT's import
libs
with ld generated ones, that end up placing the singleton thunk name
entry in the middle of idata$4.
If you are lucky, they are placed at the end of idata$4, and nothing bad
happens, as the loader must
be checking for the end of the Import Lookup Table.
With WinCE I had an app refusing to load with the nasty 193L
(ERROR_BAD_EXE_FORMAT) error.
I didn't try reproducing with Cygwin or MinGW, but the error should be
the same.
Attached is a patch to fix it. Please review and commit.
Cheers,
Pedro Alves
---
ld/ChangeLog
2006-10-29 Pedro Alves <pedro_alves@portugalmail.pt>
* pe-dll.c (make_singleton_name_thunk): Re-add the NULL terminator.
Index: pe-dll.c
===================================================================
RCS file: /cvs/src/src/ld/pe-dll.c,v
retrieving revision 1.90
diff -u -p -r1.90 pe-dll.c
--- pe-dll.c 3 Oct 2006 10:06:26 -0000 1.90
+++ pe-dll.c 29 Oct 2006 15:57:21 -0000
@@ -2080,16 +2080,17 @@ make_singleton_name_thunk (const char *i
quick_symbol (abfd, U ("_nm_thnk_"), import, "", id4, BSF_GLOBAL, 0);
quick_symbol (abfd, U ("_nm_"), import, "", UNDSEC, BSF_GLOBAL, 0);
- bfd_set_section_size (abfd, id4, PE_IDATA4_SIZE);
- d4 = xmalloc (PE_IDATA4_SIZE);
+ /* We need space for the real thunk and for the null terminator. */
+ bfd_set_section_size (abfd, id4, PE_IDATA4_SIZE * 2);
+ d4 = xmalloc (PE_IDATA4_SIZE * 2);
id4->contents = d4;
- memset (d4, 0, PE_IDATA4_SIZE);
+ memset (d4, 0, PE_IDATA4_SIZE * 2);
quick_reloc (abfd, 0, BFD_RELOC_RVA, 2);
save_relocs (id4);
bfd_set_symtab (abfd, symtab, symptr);
- bfd_set_section_contents (abfd, id4, d4, 0, PE_IDATA4_SIZE);
+ bfd_set_section_contents (abfd, id4, d4, 0, PE_IDATA4_SIZE * 2);
bfd_make_readable (abfd);
return abfd;