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: OndÅej BÃlka <neleai at seznam dot cz>
- To: Vinitha Vijayan <vinitha dot vijayann at gmail dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 8 Sep 2013 20:16:11 +0200
- 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>
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 *));