On Fri, 06 Jun 2014 07:35:22 +0900
Yoshihiro YUNOMAE <yoshihiro.yunomae...@hitachi.com> wrote:
> Current new_instance_create() implements just two fail paths for four
> allocation operations. So, it can induce memory leak if new instance
> creation failed. This patch fixes it by defining all fail paths and
> freeing allocated memories appropriately.
>
> Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae...@hitachi.com>
> Cc: Steven Rostedt <rost...@goodmis.org>
> Cc: Ingo Molnar <mi...@redhat.com>
> Cc: linux-kernel@vger.kernel.org
> ---
> kernel/trace/trace.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 473eb68..bbd86d2 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6277,7 +6277,7 @@ static int new_instance_create(const char *name)
> goto out_free_tr;
>
> if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL))
> - goto out_free_tr;
> + goto out_free_tr_name;
>
> cpumask_copy(tr->tracing_cpumask, cpu_all_mask);
>
> @@ -6291,16 +6291,16 @@ static int new_instance_create(const char *name)
> INIT_LIST_HEAD(&tr->events);
>
> if (allocate_trace_buffers(tr, trace_buf_size) < 0)
> - goto out_free_tr;
> + goto out_free_cpumask_var;
>
> tr->dir = debugfs_create_dir(name, trace_instance_dir);
> if (!tr->dir)
> - goto out_free_tr;
> + goto out_free_trace_buffers;
>
> ret = event_trace_add_tracer(tr->dir, tr);
> if (ret) {
> debugfs_remove_recursive(tr->dir);
> - goto out_free_tr;
> + goto out_free_trace_buffers;
> }
>
> init_tracer_debugfs(tr, tr->dir);
> @@ -6311,18 +6311,23 @@ static int new_instance_create(const char *name)
>
> return 0;
>
> - out_free_tr:
> - if (tr->trace_buffer.buffer)
> - ring_buffer_free(tr->trace_buffer.buffer);
> + out_free_trace_buffers:
> + ring_buffer_free(tr->trace_buffer.buffer);
> + free_percpu(tr->trace_buffer.data);
> +#ifdef CONFIG_TRACER_MAX_TRACE
> + ring_buffer_free(tr->max_buffer.buffer);
> + free_percpu(tr->max_buffer.data);
> +#endif
I think we need a free_trace_buffer() that complements
allocate_trace_buffer().
-- Steve
> + out_free_cpumask_var:
> free_cpumask_var(tr->tracing_cpumask);
> + out_free_tr_name:
> kfree(tr->name);
> + out_free_tr:
> kfree(tr);
> -
> out_unlock:
> mutex_unlock(&trace_types_lock);
>
> return ret;
> -
> }
>
> static int instance_delete(const char *name)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
