[PATCH 0/1] Fix a small memory leak in latency histograms

[PATCH 0/1] Fix a small memory leak in latency histograms

[Carsten Emde]

Hi Sebastian,

while we are at it ...

Here comes a fix of a small memory leak in latency histograms.

	-Carsten.

[PATCH 1/1] tracing/latency_hist Fix memory leak

[Carsten Emde]

[-- Attachment #1: kernel-trace-latency-hist-fix-memory-leak.patch --]
[-- Type: text/plain, Size: 707 bytes --]

The index_ptr memory that is allocated when printout is started
currently is only returned when the printout is stopped
prematurely. It is not returned when the printout regularly
finishes. Fix this memory leak.

Signed-off-by: Carsten Emde <C.Emde@osadl.org>

Index: linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
===================================================================
--- linux-3.12.10-rt15-somedebug.orig/kernel/trace/latency_hist.c
+++ linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
@@ -313,6 +313,7 @@ static void *l_next(struct seq_file *m, 
 
 	if (++*pos >= MAX_ENTRY_NUM) {
 		atomic_inc(&my_hist->hist_mode);
+		kfree(p);
 		return NULL;
 	}
 	*index_ptr = *pos;

Re: [PATCH 1/1] tracing/latency_hist Fix memory leak

[Sebastian Andrzej Siewior]

On 02/14/2014 06:26 PM, Carsten Emde wrote:
> The index_ptr memory that is allocated when printout is started
> currently is only returned when the printout is stopped
> prematurely. It is not returned when the printout regularly
> finishes. Fix this memory leak.
> 
> Signed-off-by: Carsten Emde <C.Emde@osadl.org>
> 
> Index: linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
> ===================================================================
> --- linux-3.12.10-rt15-somedebug.orig/kernel/trace/latency_hist.c
> +++ linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
> @@ -313,6 +313,7 @@ static void *l_next(struct seq_file *m, 
>  
>  	if (++*pos >= MAX_ENTRY_NUM) {
>  		atomic_inc(&my_hist->hist_mode);
> +		kfree(p);
>  		return NULL;
>  	}
>  	*index_ptr = *pos;

Sure on that? If I look at seq_read() I see that there is that ->stop()
is always called after ->start() / ->next() before returning to caller.
Based on this I would say that this patach will introduce a double free
of p.

> 
Sebastian

Re: [PATCH 1/1] tracing/latency_hist Fix memory leak

[Carsten Emde]

On 02/14/2014 08:59 PM, Sebastian Andrzej Siewior wrote:
> On 02/14/2014 06:26 PM, Carsten Emde wrote:
>> The index_ptr memory that is allocated when printout is started
>> currently is only returned when the printout is stopped
>> prematurely. It is not returned when the printout regularly
>> finishes. Fix this memory leak.
>>
>> Signed-off-by: Carsten Emde <C.Emde@osadl.org>
>>
>> Index: linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
>> ===================================================================
>> --- linux-3.12.10-rt15-somedebug.orig/kernel/trace/latency_hist.c
>> +++ linux-3.12.10-rt15-somedebug/kernel/trace/latency_hist.c
>> @@ -313,6 +313,7 @@ static void *l_next(struct seq_file *m,
>>
>>   	if (++*pos >= MAX_ENTRY_NUM) {
>>   		atomic_inc(&my_hist->hist_mode);
>> +		kfree(p);
>>   		return NULL;
>>   	}
>>   	*index_ptr = *pos;
>
> Sure on that? If I look at seq_read() I see that there is that ->stop()
> is always called after ->start() / ->next() before returning to caller.
> Based on this I would say that this patach will introduce a double free
> of p.
Some of the farm systems enable different levels of run time debug 
features including kmemleak in two of them. These two machines regularly 
crashed with kmemleak alloc overflow, because the system detected so 
many memleaks that there was no more memory available to store all of 
them. Since the objects were so small, the leak was not measurable in 
terms of memory exhaustion. I then investigated the kmemleak objects, 
and they clearly pointed to the index_ptr memory. This was the only 
place I found, and kmemleak no longer complaint or crashed after 
inserting this kfree(). I then patched the other farm systems some time 
ago where it never never did any harm. I needed to find a solution to 
the kmemleak machines, since otherwise I wouldn't have a usable tool to 
search for other memleaks. But I know that I should have done better. So 
please drop this patch and wait, until I find some time to more 
carefully analyze the entire situation.

	-Carsten.