Skip to content

Commit

Permalink
tracing: fix memory leak in trace_stat
Browse files Browse the repository at this point in the history
If the function profiler does not have any items recorded and one were
to cat the function stat file, the kernel would take a BUG with a NULL
pointer dereference.

Looking further into this, I found that returning NULL from stat_start
did not stop the stat logic, and would later call stat_next. This breaks
from the way seq_file works, so I looked into fixing the stat code.

This is where I noticed that the last next_entry is never freed.
It is allocated, and if the stat_next returns NULL, the code breaks out
of the loop, unlocks the mutex and exits. We never link the next_entry
nor do we free it. Thus it is a real memory leak.

This patch rearranges the code a bit to not only fix the memory leak,
but also to act more like seq_file where nothing is printed if there
is nothing to print. That is, stat_start returns NULL.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
  • Loading branch information
Steven Rostedt committed Mar 24, 2009
1 parent 45b9560 commit 0983352
Showing 1 changed file with 13 additions and 10 deletions.
23 changes: 13 additions & 10 deletions kernel/trace/trace_stat.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static int stat_seq_init(struct tracer_stat_session *session)
{
struct trace_stat_list *iter_entry, *new_entry;
struct tracer_stat *ts = session->ts;
void *prev_stat;
void *stat;
int ret = 0;
int i;

Expand All @@ -85,6 +85,10 @@ static int stat_seq_init(struct tracer_stat_session *session)
if (!ts->stat_cmp)
ts->stat_cmp = dummy_cmp;

stat = ts->stat_start();
if (!stat)
goto exit;

/*
* The first entry. Actually this is the second, but the first
* one (the stat_list head) is pointless.
Expand All @@ -99,26 +103,27 @@ static int stat_seq_init(struct tracer_stat_session *session)

list_add(&new_entry->list, &session->stat_list);

new_entry->stat = ts->stat_start();
prev_stat = new_entry->stat;
new_entry->stat = stat;

/*
* Iterate over the tracer stat entries and store them in a sorted
* list.
*/
for (i = 1; ; i++) {
stat = ts->stat_next(stat, i);

/* End of insertion */
if (!stat)
break;

new_entry = kmalloc(sizeof(struct trace_stat_list), GFP_KERNEL);
if (!new_entry) {
ret = -ENOMEM;
goto exit_free_list;
}

INIT_LIST_HEAD(&new_entry->list);
new_entry->stat = ts->stat_next(prev_stat, i);

/* End of insertion */
if (!new_entry->stat)
break;
new_entry->stat = stat;

list_for_each_entry(iter_entry, &session->stat_list, list) {

Expand All @@ -137,8 +142,6 @@ static int stat_seq_init(struct tracer_stat_session *session)
break;
}
}

prev_stat = new_entry->stat;
}
exit:
mutex_unlock(&session->stat_mutex);
Expand Down

0 comments on commit 0983352

Please sign in to comment.