This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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 *));


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]