Re: [PATCH][BZ #15859] Memory leak detected in _dl_map_object_deps()

[Vinitha Vijayan <vinitha dot vijayann at gmail dot com>]

Hi,
The patch is recreated as per the comments and attached.

Regards,
Vinitha Vijayan
Sony India Software Centre Pvt Ltd

On Tue, Sep 10, 2013 at 7:07 PM, Vinitha Vijayan
<vinitha.vijayann@gmail.com> wrote:
> Thank you for the comments.
> I have gone through the link
> https://sourceware.org/glibc/wiki/Style_and_Conventions .
> I understand and will be  taking care of this from next time.
>
> Regards,
> Vinitha
>
> On Sun, Sep 8, 2013 at 11:46 PM, OndÅej BÃlka <neleai@seznam.cz> wrote:
>> On Thu, Sep 05, 2013 at 12:02:29PM +0530, Vinitha Vijayan wrote:
>>> Hi,
>>>  Memory leak observed in the _dl_map_object_deps().
>>>  The following malloc() causes the leak.
>>>
>> Change looks ok, if you want to submit more patches you should learn
>> conventions, see below.
>>
>>> ...
>>>             l_reldeps = malloc (sizeof (*l_reldeps)
>>>                                 + map->l_reldepsmax
>>>                                   * sizeof (struct link_map *));
>>> ...
>>>
>>> The issue can be reproduced using a minimal test scenario which is shown below
>>> ...
>>>    * libX.so
>>>        \--------> libA.so
>>>        \--------> libB.so
>>>        \--------> libC.so
>>>
>>>    * libA.so
>>>        \-------> libB.so
>>>        \........................> libC.so ( relocation dependency)
>>>
>>>    * libB.so
>>>        \-------> libC.so
>>>  * main application
>>>   {
>>>    ...
>>>    dlopen(libX.so);
>>>    ...
>>>    dlopen(libA.so)
>>>    }
>>> ...
>>> This happens because of duplicate declaration of pointer l_reldeps as
>>> shown below
>>> ...
>>>   struct link_map_reldeps *l_reldeps = NULL;       ===> HERE
>>>   if (map->l_reldeps != NULL)
>>>     {
>>>       for (i = 1; i < nlist; ++i)
>>>         map->l_searchlist.r_list[i]->
>>>
>>> l_reserved = 1;
>>>       struct link_map **list = &map->l_reldeps->list[0];
>>>       for (i = 0; i < map->l_reldeps->act; ++i)
>>>         if (list[i]->l_reserved)
>>>           {
>>>             /* Need to allocate new array of relocation dependencies.  */
>>>             struct link_map_reldeps *l_reldeps;  ===>  HERE
>>>             l_reldeps = malloc (sizeof (*l_reldeps)
>>>                                 + map->l_reldepsmax
>>>                                   * sizeof (struct link_map *));
>>> ...
>>>
>>> The fix is to remove the duplicate declaration inside the if loop.
>>>
>>> Patch:
>>>
>>> Index: b/elf/dl-deps.c
>>> ===================================================================
>>> --- a/elf/dl-deps.c
>>> +++ b/elf/dl-deps.c
>>> @@ -595,7 +595,6 @@ Filters not supported with LD_TRACE_PREL
>>>
>>>   if (list[i]->l_reserved)
>>>    {
>>>      /* Need to allocate new array of relocation dependencies.  */
>>> -    struct link_map_reldeps *l_reldeps;
>>>      l_reldeps = malloc (sizeof (*l_reldeps)
>>>   + map->l_reldepsmax
>>>
>>>    * sizeof (struct link_map *));
>>>
>>> Regards,
>>> Vinitha Vijayan
>>> Sony India Software Centre Pvt Ltd.
>>
>> A formatting of patch looks off, also you need to write changelog entry.
>> see http://sourceware.org/glibc/wiki/Style_and_Conventions
>>
>> For this change correct format is following:
>>
>> 2013-09-08  Vinitha Vijayan <vinitha.vijayann@gmail.com>
>>
>>         [BZ #15859]
>>         * elf/dl-deps.c (_dl_map_object_deps): Remove duplicate declaration.
>>
>> diff --git a/elf/dl-deps.c b/elf/dl-deps.c
>> index 1c36f50..6652f6d 100644
>> --- a/elf/dl-deps.c
>> +++ b/elf/dl-deps.c
>> @@ -595,7 +595,6 @@ Filters not supported with LD_TRACE_PRELINKING"));
>>         if (list[i]->l_reserved)
>>           {
>>             /* Need to allocate new array of relocation dependencies.  */
>> -           struct link_map_reldeps *l_reldeps;
>>             l_reldeps = malloc (sizeof (*l_reldeps)
>>                                 + map->l_reldepsmax
>>                                   * sizeof (struct link_map *));

Attachment: glibc-fix-BZ15859.patch
Description: Binary data