Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [for-next][PATCH 6/6] tracing: Fix leak of ring buffer data when new instances creation fails | Date | Tue, 10 Jun 2014 14:25:21 +0900 |
| |
On Fri, 06 Jun 2014 12:30:40 -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> > > Yoshihiro Yunomae reported that the ring buffer data for a trace > instance does not get properly cleaned up when it fails. He proposed > a patch that manually cleaned the data up and addad a bunch of labels. > The labels are not needed because all trace array is allocated with > a kzalloc which initializes it to 0 and all kfree()s can take a NULL > pointer and will ignore it. > > Adding a new helper function free_trace_buffers() that can also take > null buffers to free the buffers that were allocated by > allocate_trace_buffers(). > > Link: http://lkml.kernel.org/r/20140605223522.32311.31664.stgit@yunodevel > > Reported-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > --- > kernel/trace/trace.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index e29edee1542a..26cfff38e2ab 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -6232,6 +6232,25 @@ static int allocate_trace_buffers(struct trace_array *tr, int size) > return 0; > } > > +static void free_trace_buffers(struct trace_array *tr) > +{ > + if (!tr) > + return; > + > + if (tr->trace_buffer.buffer) { > + ring_buffer_free(tr->trace_buffer.buffer); > + tr->trace_buffer.buffer = NULL; > + free_percpu(tr->trace_buffer.data); > + } > + > +#ifdef CONFIG_TRACER_MAX_TRACE > + if (tr->max_buffer.buffer) { > + ring_buffer_free(tr->max_buffer.buffer); > + tr->max_buffer.buffer = NULL;
Hmm.. why doesn't it free tr->max_buffer.data here? And I think it's better to reset *_buffer.data to NULL also - maybe by adding another helper function free_trace_buffer()..
Thanks, Namhyung
> + } > +#endif > +} > + > static int new_instance_create(const char *name) > { > struct trace_array *tr; > @@ -6290,8 +6309,7 @@ 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); > + free_trace_buffers(tr); > free_cpumask_var(tr->tracing_cpumask); > kfree(tr->name); > kfree(tr);
| |