Hi, while working on free software ports to Windows CE I noticed two bugs in binutils' BFD support for some PE files (for example, kmail-mobile.exe built with MSVC). Fixes for both are included below. Copyright assignments by g10 Code GmbH are on file at the FSF. If you need anything else, just let me know. The file that triggers these bugs can be found at:
ftp://ftp.g10code.com/people/marcus/kmail-mobile-binutils-test.exe.gz The first issue concerns import tables where the thunk table is found in a different section. In this case, BFD tries to load DATASIZE bytes from that section at the beginning of the thunk array, but DATASIZE is the remaining bytes in the import table section starting from the beginning of the import table. This number is in no way related to the size of the thunk table or the section in which this thunk table is to be found. So, my patch introduces a new size, limit_size, which is correctly calculated and used in the appropriate places. Without the patch, no import symbols would be shown for kmail-mobile.exe (Visual Studio 2008, Windows CE), because the data section is in this case not large enough to read DATASIZE bytes from it. With the patch, loading LIMIT_SIZE bytes succeeds and all import symbols are shown correctly. The second issue concerns the support for compressed pdata support for Windows CE. In this code is a simple memory leak. First, the whole section is malloced and copied to TDATA, then immediately TDATA is overwritten with a much smaller buffer to which only the required section data is copied, leaking memory in the size of the section for each entry in the table. For kmail-mobile.exe, the table is very large (hundreds of entries), leaking Gigabytes of memory quickly and basically creating denial of service attack. Thanks a lot, Marcus Brinkmann mar...@g10code.com -- g10 Code GmbH http://g10code.com AmtsGer. Wuppertal HRB 14459 Hüttenstr. 61 Geschäftsführung Werner Koch D-40699 Erkrath -=- The GnuPG Experts -=- USt-Id DE215605608
2010-09-03 Marcus Brinkmann <mar...@g10code.de> * peXXigen.c (pe_print_idata): Use correct size limit to load thunk table from different section. Index: peXXigen.c =================================================================== RCS file: /cvs/src/src/bfd/peXXigen.c,v retrieving revision 1.64 diff -u -p -r1.64 peXXigen.c --- peXXigen.c 27 Jun 2010 04:07:53 -0000 1.64 +++ peXXigen.c 2 Sep 2010 23:02:08 -0000 @@ -1226,6 +1226,7 @@ pe_print_idata (bfd * abfd, void * vfile bfd_size_type ft_datasize; int ft_idx; int ft_allocated = 0; + bfd_size_type limit_size; fprintf (file, _("\tvma: Hint/Ord Member-Name Bound-To\n")); @@ -1262,17 +1263,19 @@ pe_print_idata (bfd * abfd, void * vfile { ft_data = data; ft_idx = first_thunk - adj; + limit_size = section->size - ft_idx; } else { ft_idx = first_thunk - (ft_section->vma - extra->ImageBase); - ft_data = (bfd_byte *) bfd_malloc (datasize); + limit_size = ft_section->size - ft_idx; + ft_data = (bfd_byte *) bfd_malloc (limit_size); if (ft_data == NULL) continue; - /* Read datasize bfd_bytes starting at offset ft_idx. */ + /* Read limit_size bfd_bytes starting at offset ft_idx. */ if (! bfd_get_section_contents - (abfd, ft_section, ft_data, (bfd_vma) ft_idx, datasize)) + (abfd, ft_section, ft_data, (bfd_vma) ft_idx, limit_size)) { free (ft_data); continue; @@ -1285,7 +1288,7 @@ pe_print_idata (bfd * abfd, void * vfile /* Print HintName vector entries. */ #ifdef COFF_WITH_pex64 - for (j = 0; j < datasize; j += 8) + for (j = 0; j < limit_size; j += 8) { unsigned long member = bfd_get_32 (abfd, data + idx + j); unsigned long member_high = bfd_get_32 (abfd, data + idx + j + 4); @@ -1316,7 +1319,7 @@ pe_print_idata (bfd * abfd, void * vfile fprintf (file, "\n"); } #else - for (j = 0; j < datasize; j += 4) + for (j = 0; j < limit_size; j += 4) { unsigned long member = bfd_get_32 (abfd, data + idx + j);
2010-09-03 Marcus Brinkmann <mar...@g10code.de> * peXXigen.c (_bfd_XX_print_ce_compressed_pdata): Fix memory leak. Index: peXXigen.c =================================================================== RCS file: /cvs/src/src/bfd/peXXigen.c,v retrieving revision 1.64 diff -u -p -r1.64 peXXigen.c --- peXXigen.c 27 Jun 2010 04:07:53 -0000 1.64 +++ peXXigen.c 2 Sep 2010 23:02:08 -0000 @@ -1828,7 +1831,6 @@ _bfd_XX_print_ce_compressed_pdata (bfd * bfd_vma other_data; bfd_vma prolog_length, function_length; int flag32bit, exception_flag; - bfd_byte *tdata = 0; asection *tsection; if (i + PDATA_ROW_SIZE > stop) @@ -1860,36 +1862,29 @@ _bfd_XX_print_ce_compressed_pdata (bfd * if (tsection && coff_section_data (abfd, tsection) && pei_section_data (abfd, tsection)) { - if (bfd_malloc_and_get_section (abfd, tsection, & tdata)) - { - int xx = (begin_addr - 8) - tsection->vma; + int xx = (begin_addr - 8) - tsection->vma; + bfd_byte *tdata = 0; - tdata = (bfd_byte *) bfd_malloc (8); - if (bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8)) + tdata = (bfd_byte *) bfd_malloc (8); + if (tdata && bfd_get_section_contents (abfd, tsection, tdata, (bfd_vma) xx, 8)) + { + bfd_vma eh, eh_data; + + eh = bfd_get_32 (abfd, tdata); + eh_data = bfd_get_32 (abfd, tdata + 4); + fprintf (file, "%08x ", (unsigned int) eh); + fprintf (file, "%08x", (unsigned int) eh_data); + if (eh != 0) { - bfd_vma eh, eh_data; - - eh = bfd_get_32 (abfd, tdata); - eh_data = bfd_get_32 (abfd, tdata + 4); - fprintf (file, "%08x ", (unsigned int) eh); - fprintf (file, "%08x", (unsigned int) eh_data); - if (eh != 0) - { - const char *s = my_symbol_for_address (abfd, eh, &cache); - - if (s) - fprintf (file, " (%s) ", s); - } + const char *s = my_symbol_for_address (abfd, eh, &sym_cache); + + if (s) + fprintf (file, " (%s) ", s); } free (tdata); } - else - { - if (tdata) - free (tdata); - } } - + fprintf (file, "\n"); }
_______________________________________________ bug-binutils mailing list bug-binutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-binutils