Re: [PATCH 3/4] tracing/events: fix memory leak when unloading module

Steven Rostedt
2009-05-06T02:43:30Z

On Wed, 6 May 2009, Li Zefan wrote:

> When unloading a module, memory allocated by init_preds() and
> trace_define_field() is not freed.
>
> [ Impact: fix memory leak ]
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Nice catch!

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve

> ---
> include/linux/ftrace_event.h | 1 +
> kernel/trace/trace_events.c | 18 ++++++++++++++++++
> kernel/trace/trace_events_filter.c | 22 +++++++++++++++-------
> 3 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 5fff40c..662c1be 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -116,6 +116,7 @@ struct ftrace_event_call {
> #define MAX_FILTER_STR_VAL 128
>
> extern int init_preds(struct ftrace_event_call *call);
> +extern void destroy_preds(struct ftrace_event_call *call);
> extern int filter_match_preds(struct ftrace_event_call *call, void *rec);
> extern int filter_current_check_discard(struct ftrace_event_call *call,
> void *rec,
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 9998cc8..72d619d 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -60,6 +60,22 @@ err:
> }
> EXPORT_SYMBOL_GPL(trace_define_field);
>
> +#ifdef CONFIG_MODULES
> +
> +static void trace_destroy_fields(struct ftrace_event_call *call)
> +{
> + struct ftrace_event_field *field, *next;
> +
> + list_for_each_entry_safe(field, next, &call->fields, link) {
> + list_del(&field->link);
> + kfree(field->type);
> + kfree(field->name);
> + kfree(field);
> + }
> +}
> +
> +#endif /* CONFIG_MODULES */
> +
> static void ftrace_clear_events(void)
> {
> struct ftrace_event_call *call;
> @@ -925,6 +941,8 @@ static void trace_module_remove_events(struct module *mod)
> unregister_ftrace_event(call->event);
> debugfs_remove_recursive(call->dir);
> list_del(&call->list);
> + trace_destroy_fields(call);
> + destroy_preds(call);
> }
> }
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index ac30de3..97bd140 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -371,6 +371,20 @@ static void filter_disable_preds(struct ftrace_event_call *call)
> filter->preds[i]->fn = filter_pred_none;
> }
>
> +void destroy_preds(struct ftrace_event_call *call)
> +{
> + struct event_filter *filter = call->filter;
> + int i;
> +
> + for (i = 0; i < MAX_FILTER_PRED; i++) {
> + if (filter->preds[i])
> + filter_free_pred(filter->preds[i]);
> + }
> + kfree(filter->preds);
> + kfree(filter);
> + call->filter = NULL;
> +}
> +
> int init_preds(struct ftrace_event_call *call)
> {
> struct event_filter *filter;
> @@ -399,13 +413,7 @@ int init_preds(struct ftrace_event_call *call)
> return 0;
>
> oom:
> - for (i = 0; i < MAX_FILTER_PRED; i++) {
> - if (filter->preds[i])
> - filter_free_pred(filter->preds[i]);
> - }
> - kfree(filter->preds);
> - kfree(call->filter);
> - call->filter = NULL;
> + destroy_preds(call);
>
> return -ENOMEM;
> }
> --
> 1.5.4.rc3
>
>
>