Re: [patch bfd]: Prevent possible buffer overflow on pdata-section sorting

[Kai Tietz <ktietz70 at googlemail dot com>]

2011/4/7 Kai Tietz <ktietz70@googlemail.com>:
> 2011/4/7 Alan Modra <amodra@gmail.com>:
>> On Wed, Apr 06, 2011 at 06:50:15PM +0200, Kai Tietz wrote:
>>> Hello,
>>>
>>> this issue was reported by H. Becker to me. ?He found that the code in
>>> peXXigen.c about pdata-section sorting might cause a buffer-overrun
>>> for large pdata-data. ?By working in private allocated buffer -
>>> instead of using the pfinfo->contents - avoids this.
>>>
>>> ChangeLog
>>>
>>> 2011-04-06 ?Kai Tietz
>>>
>>> ? ? ? ? * peXXigen.c (_bfd_XXi_final_link_postscripte): Sort pdata in temporary
>>> ? ? ? ? buffer.
>>>
>>> Tested for x86_64-w64-mingw32. Ok for apply?
>>>
>>> Regards,
>>> Kai
>>
>>> Index: src/bfd/peXXigen.c
>>> ===================================================================
>>> --- src.orig/bfd/peXXigen.c ? 2010-12-21 19:33:07.000000000 +0100
>>> +++ src/bfd/peXXigen.c ? ? ? ?2011-04-06 18:19:45.945394800 +0200
>>> @@ -2459,14 +2459,22 @@ _bfd_XXi_final_link_postscript (bfd * ab
>>> ? ? ?if (sec)
>>> ? ? ? ?{
>>> ? ? ? bfd_size_type x = sec->rawsize ? sec->rawsize : sec->size;
>>
>> Since this is an output section, this should just be sec->size I
>> think. ?See section.c rawsize comment.
>
> Well, the cause for using here raw_size (I will look into section.c to
> read the comment there9 was that we need to sort without alignment. As
> it is an output-section, its size might be padded already with
> alignment fill, which shouldn't be sorted. ?But you might be right
> here that size is suitable.

Hmm, not sure. I think it makes sense to check here for raw_size. In
section.c the member size has the following documentation: "The size
of the section in octets, as it will be output. Contains a value even
if the section has no contents (e.g., the size of <<.bss>>). )".
And the rawsize memember has for output-sections the following
definition: "For output sections, rawsize holds the  section size
calculated on a previous linker relaxation pass.", which seems to be
the thing we need. It might be a way to allocate section's size, but
then sort only in range of rawsize, but not sure if this is necessary,
as on output the section alignment get applied again, isn't it?

Kai