diff mbox series

[v2] malloc: fix memleak in function muntrace

Message ID 20200420134920.93473-1-chenzefeng2@huawei.com
State Superseded
Headers
Series [v2] malloc: fix memleak in function muntrace |

Commit Message

chenzefeng April 20, 2020, 1:49 p.m. UTC 
  when we call functons as follow:
	mtrace();
	...
	muntrace();
It would cause memleak, for the mtrace malloc some memory:
	mtd = malloc(TRACE_BUFFER_SIZE);
and it would not be free. Therefor it should be freed in muntrace.

Signed-off-by: chenzefeng <chenzefeng2@huawei.com>
---
 malloc/mtrace.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Florian Weimer April 20, 2020, 2:50 p.m. UTC | #1 
* chenzefeng:

> when we call functons as follow:
> 	mtrace();
> 	...
> 	muntrace();
> It would cause memleak, for the mtrace malloc some memory:
> 	mtd = malloc(TRACE_BUFFER_SIZE);
> and it would not be free. Therefor it should be freed in muntrace.
>
> Signed-off-by: chenzefeng <chenzefeng2@huawei.com>

glibc uses copyright assignment to the FSF for contributions, and not
the DCO.

I trust this contribution is covered by Huawei's copyright assignment?

> ---
>  malloc/mtrace.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/malloc/mtrace.c b/malloc/mtrace.c
> index 7e7719df97..ab65ebe764 100644
> --- a/malloc/mtrace.c
> +++ b/malloc/mtrace.c
> @@ -365,4 +365,6 @@ muntrace (void)
>  
>    fprintf (f, "= End\n");
>    fclose (f);
> +  if (malloc_trace_buffer != NULL)
> +    free (malloc_trace_buffer);
>  }

I think you can call free unconditionally.
Zack Weinberg April 20, 2020, 4:15 p.m. UTC | #2 
On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> * chenzefeng:
> > +  if (malloc_trace_buffer != NULL)
> > +    free (malloc_trace_buffer);
> >  }
>
> I think you can call free unconditionally.

I was going to say the same thing.  (free(NULL) is specified to do nothing.)

Also, it would be good to set malloc_trace_buffer to NULL after calling free.

Also, __release_libc_mem should be calling muntrace (after calling
__libc_freeres).

zw
Florian Weimer April 20, 2020, 4:19 p.m. UTC | #3 
* Zack Weinberg:

> On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>> * chenzefeng:
>> > +  if (malloc_trace_buffer != NULL)
>> > +    free (malloc_trace_buffer);
>> >  }
>>
>> I think you can call free unconditionally.
>
> I was going to say the same thing.  (free(NULL) is specified to do nothing.)
>
> Also, it would be good to set malloc_trace_buffer to NULL after
> calling free.

This introduces an asymmetry with the handling of f.  The existing
code is fine either way.
Zack Weinberg April 20, 2020, 6:19 p.m. UTC | #4 
On Mon, Apr 20, 2020 at 12:19 PM Florian Weimer <fw@deneb.enyo.de> wrote:
> * Zack Weinberg:
> > On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >> * chenzefeng:
> >> > +  if (malloc_trace_buffer != NULL)
> >> > +    free (malloc_trace_buffer);
> >> >  }
> >>
> >> I think you can call free unconditionally.
> >
> > I was going to say the same thing.  (free(NULL) is specified to do nothing.)
> >
> > Also, it would be good to set malloc_trace_buffer to NULL after
> > calling free.
>
> This introduces an asymmetry with the handling of f.

f is a local variable.  muntrace does set mallstream to NULL.

zw
Florian Weimer April 21, 2020, 9:11 a.m. UTC | #5 
* Zack Weinberg:

> On Mon, Apr 20, 2020 at 12:19 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>> * Zack Weinberg:
>> > On Mon, Apr 20, 2020 at 10:50 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>> >> * chenzefeng:
>> >> > +  if (malloc_trace_buffer != NULL)
>> >> > +    free (malloc_trace_buffer);
>> >> >  }
>> >>
>> >> I think you can call free unconditionally.
>> >
>> > I was going to say the same thing.  (free(NULL) is specified to do nothing.)
>> >
>> > Also, it would be good to set malloc_trace_buffer to NULL after
>> > calling free.
>>
>> This introduces an asymmetry with the handling of f.
>
> f is a local variable.  muntrace does set mallstream to NULL.

Oh in this case, malloc_trace_buffer should be reset as well.
diff mbox series

Patch

diff --git a/malloc/mtrace.c b/malloc/mtrace.c
index 7e7719df97..ab65ebe764 100644
--- a/malloc/mtrace.c
+++ b/malloc/mtrace.c
@@ -365,4 +365,6 @@  muntrace (void)
 
   fprintf (f, "= End\n");
   fclose (f);
+  if (malloc_trace_buffer != NULL)
+    free (malloc_trace_buffer);
 }