Re: [qboosh@pld-linux.org: binutils 2.14.90.0.8 - ld crash whencross-linking PE DLL on 64-bit archs]

[Nick Clifton <nickc at redhat dot com>]

Hi H.J.  Hi Jakub,

> SEGV appears inside malloc(), after corruption of some malloc structs.
> Using ElectricFence I found that ld writes past allocated area in
> fill_edata() function (ld/pe-dll.c).
>
> edata_d is pointer to allocated edata_sz bytes, where edata_sz is
> calculated by:
>
> |   /* OK, now we can allocate some memory.  */
> |   edata_sz = (40                                /* directory */
> |               + 4 * export_table_size           /* addresses */
> |               + 4 * count_exported_byname       /* name ptrs */
> |               + 2 * count_exported_byname       /* ordinals */
> |               + name_table_size + strlen (dll_name) + 1);
>
> but then pointers to edata areas are calculated inconsequently:
>
> |   unsigned char *edirectory;
> |   unsigned long *eaddresses;
> |   unsigned long *enameptrs;
> |   unsigned short *eordinals;
> |   unsigned char *enamestr;
> [...]
> |   edata_d = xmalloc (edata_sz);
> | 
> |   /* Note use of array pointer math here.  */
> |   edirectory = edata_d;
> |   eaddresses = (unsigned long *) (edata_d + 40);
> |   enameptrs = eaddresses + export_table_size;
> |   eordinals = (unsigned short *) (enameptrs + count_exported_byname);
> |   enamestr = (char *) (eordinals + count_exported_byname);
>
> These pointers are wrong if sizeof(unsigned long)!=4 or
> sizeof(unsigned short)!=2.
> On alpha-linux and amd64-linux it's the first case (long is 8-byte long).
>
> I'm attaching the patch I've used - it changes unsinged long to uint32_t
> in above pointers. It works for me on alpha-linux and amd64-linux, doesn't
> break anything on x86-linux and sparc-linux.

> I don't know what is binutils policy on <stdint.h> - is using uint32_t
> OK, or binutils has some other name for exactly 32-bit long type.

Sorry, no it does not.

> unsigned shorts above probably should be changed to uint16_t (or
> equivalent), but I didn't touch it as all archs I'm using have 16-bit
> unsigned short.

The patch is a good start, but it does not go quite far enough.  The
16-bit types really should be forced to be exactly 16-bits as well.

As for using stdint.h - that is OK, provided that it is checked for by
configure and then only used if HAVE_STDINT_H is defined.

If you would care to resubmit the patch with these changes made then I
would be happy to accept it.

Cheers
        Nick