diff options
author | Steven Rostedt <srostedt@redhat.com> | 2009-03-21 02:44:50 -0400 |
---|---|---|
committer | Steven Rostedt <srostedt@redhat.com> | 2009-03-24 09:07:35 -0400 |
commit | 098335215a4921a8a54193829eaed602dca24df5 (patch) | |
tree | 86226d7e4229f00e467b5e6ed7f048a2061c3042 /kernel/trace | |
parent | 45b9560895b07a4a09d55d49235c984db512c5aa (diff) |
tracing: fix memory leak in trace_stat
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>
Diffstat (limited to 'kernel/trace')
-rw-r--r-- | kernel/trace/trace_stat.c | 23 |
1 files changed, 13 insertions, 10 deletions
diff --git a/kernel/trace/trace_stat.c b/kernel/trace/trace_stat.c index 39310e3434ee..f71b85b22cfe 100644 --- a/kernel/trace/trace_stat.c +++ b/kernel/trace/trace_stat.c | |||
@@ -75,7 +75,7 @@ static int stat_seq_init(struct tracer_stat_session *session) | |||
75 | { | 75 | { |
76 | struct trace_stat_list *iter_entry, *new_entry; | 76 | struct trace_stat_list *iter_entry, *new_entry; |
77 | struct tracer_stat *ts = session->ts; | 77 | struct tracer_stat *ts = session->ts; |
78 | void *prev_stat; | 78 | void *stat; |
79 | int ret = 0; | 79 | int ret = 0; |
80 | int i; | 80 | int i; |
81 | 81 | ||
@@ -85,6 +85,10 @@ static int stat_seq_init(struct tracer_stat_session *session) | |||
85 | if (!ts->stat_cmp) | 85 | if (!ts->stat_cmp) |
86 | ts->stat_cmp = dummy_cmp; | 86 | ts->stat_cmp = dummy_cmp; |
87 | 87 | ||
88 | stat = ts->stat_start(); | ||
89 | if (!stat) | ||
90 | goto exit; | ||
91 | |||
88 | /* | 92 | /* |
89 | * The first entry. Actually this is the second, but the first | 93 | * The first entry. Actually this is the second, but the first |
90 | * one (the stat_list head) is pointless. | 94 | * one (the stat_list head) is pointless. |
@@ -99,14 +103,19 @@ static int stat_seq_init(struct tracer_stat_session *session) | |||
99 | 103 | ||
100 | list_add(&new_entry->list, &session->stat_list); | 104 | list_add(&new_entry->list, &session->stat_list); |
101 | 105 | ||
102 | new_entry->stat = ts->stat_start(); | 106 | new_entry->stat = stat; |
103 | prev_stat = new_entry->stat; | ||
104 | 107 | ||
105 | /* | 108 | /* |
106 | * Iterate over the tracer stat entries and store them in a sorted | 109 | * Iterate over the tracer stat entries and store them in a sorted |
107 | * list. | 110 | * list. |
108 | */ | 111 | */ |
109 | for (i = 1; ; i++) { | 112 | for (i = 1; ; i++) { |
113 | stat = ts->stat_next(stat, i); | ||
114 | |||
115 | /* End of insertion */ | ||
116 | if (!stat) | ||
117 | break; | ||
118 | |||
110 | new_entry = kmalloc(sizeof(struct trace_stat_list), GFP_KERNEL); | 119 | new_entry = kmalloc(sizeof(struct trace_stat_list), GFP_KERNEL); |
111 | if (!new_entry) { | 120 | if (!new_entry) { |
112 | ret = -ENOMEM; | 121 | ret = -ENOMEM; |
@@ -114,11 +123,7 @@ static int stat_seq_init(struct tracer_stat_session *session) | |||
114 | } | 123 | } |
115 | 124 | ||
116 | INIT_LIST_HEAD(&new_entry->list); | 125 | INIT_LIST_HEAD(&new_entry->list); |
117 | new_entry->stat = ts->stat_next(prev_stat, i); | 126 | new_entry->stat = stat; |
118 | |||
119 | /* End of insertion */ | ||
120 | if (!new_entry->stat) | ||
121 | break; | ||
122 | 127 | ||
123 | list_for_each_entry(iter_entry, &session->stat_list, list) { | 128 | list_for_each_entry(iter_entry, &session->stat_list, list) { |
124 | 129 | ||
@@ -137,8 +142,6 @@ static int stat_seq_init(struct tracer_stat_session *session) | |||
137 | break; | 142 | break; |
138 | } | 143 | } |
139 | } | 144 | } |
140 | |||
141 | prev_stat = new_entry->stat; | ||
142 | } | 145 | } |
143 | exit: | 146 | exit: |
144 | mutex_unlock(&session->stat_mutex); | 147 | mutex_unlock(&session->stat_mutex); |