This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #15859] Memory leak detected in _dl_map_object_deps()
- From: Vinitha Vijayan <vinitha dot vijayann at gmail dot com>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 10 Sep 2013 19:07:14 +0530
- Subject: Re: [PATCH][BZ #15859] Memory leak detected in _dl_map_object_deps()
- Authentication-results: sourceware.org; auth=none
- References: <CABcFW_gtNfpffLHWoqtg51Dh4sRprtnVAHPGV+oub8VvCSQq+Q at mail dot gmail dot com> <20130908181611 dot GA27001 at domone dot kolej dot mff dot cuni dot cz>
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 *));