* [PATCH 1/2] tracing/filters: free filter_string in destroy_preds() @ 2009-06-16 8:39 Li Zefan 2009-06-16 8:39 ` [PATCH 2/2] tracing/filters: fix race between filter setting and module unload Li Zefan 2009-06-17 11:13 ` [tip:tracing/urgent] tracing/filters: free filter_string in destroy_preds() tip-bot for Li Zefan 0 siblings, 2 replies; 4+ messages in thread From: Li Zefan @ 2009-06-16 8:39 UTC (permalink / raw) To: Steven Rostedt, Ingo Molnar; +Cc: Frederic Weisbecker, Tom Zanussi, LKML filter->filter_string is not freed when unloading a module: # insmod trace-events-sample.ko # echo "bar < 100" > /mnt/tracing/events/sample/foo_bar/filter # rmmod trace-events-sample.ko [ Impact: fix memory leak when unloading module ] Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> --- kernel/trace/trace_events_filter.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index b24ab0e..d9f01c1 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c @@ -381,6 +381,7 @@ void destroy_preds(struct ftrace_event_call *call) filter_free_pred(filter->preds[i]); } kfree(filter->preds); + kfree(filter->filter_string); kfree(filter); call->filter = NULL; } -- 1.5.4.rc3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] tracing/filters: fix race between filter setting and module unload 2009-06-16 8:39 [PATCH 1/2] tracing/filters: free filter_string in destroy_preds() Li Zefan @ 2009-06-16 8:39 ` Li Zefan 2009-06-17 11:13 ` [tip:tracing/urgent] " tip-bot for Li Zefan 2009-06-17 11:13 ` [tip:tracing/urgent] tracing/filters: free filter_string in destroy_preds() tip-bot for Li Zefan 1 sibling, 1 reply; 4+ messages in thread From: Li Zefan @ 2009-06-16 8:39 UTC (permalink / raw) To: Steven Rostedt, Ingo Molnar; +Cc: Frederic Weisbecker, Tom Zanussi, LKML Module unload is protected by event_mutex, while setting filter is protected by filter_mutex. This leads to the race: echo 'bar == 0 || bar == 10' \ | > sample/filter | | insmod sample.ko add_pred("bar == 0") | -> n_preds == 1 | add_pred("bar == 100") | -> n_preds == 2 | | rmmod sample.ko | insmod sample.ko add_pred("&&") | -> n_preds == 1 (should be 3) | Now event->filter->preds is corrupted. An then when filter_match_preds() is called, the WARN_ON() in it will be triggered. To avoid the race, we remove filter_mutex, and replace it with event_mutex. [ fix race that can cause WARN_ON (even sth worse) when filtering events ] Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> --- kernel/trace/trace_events_filter.c | 27 ++++++++++----------------- 1 files changed, 10 insertions(+), 17 deletions(-) diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index d9f01c1..936c621 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c @@ -27,8 +27,6 @@ #include "trace.h" #include "trace_output.h" -static DEFINE_MUTEX(filter_mutex); - enum filter_op_ids { OP_OR, @@ -294,12 +292,12 @@ void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s) { struct event_filter *filter = call->filter; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (filter->filter_string) trace_seq_printf(s, "%s\n", filter->filter_string); else trace_seq_printf(s, "none\n"); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); } void print_subsystem_event_filter(struct event_subsystem *system, @@ -307,12 +305,12 @@ void print_subsystem_event_filter(struct event_subsystem *system, { struct event_filter *filter = system->filter; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (filter->filter_string) trace_seq_printf(s, "%s\n", filter->filter_string); else trace_seq_printf(s, "none\n"); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); } static struct ftrace_event_field * @@ -434,7 +432,6 @@ static void filter_free_subsystem_preds(struct event_subsystem *system) filter->n_preds = 0; } - mutex_lock(&event_mutex); list_for_each_entry(call, &ftrace_events, list) { if (!call->define_fields) continue; @@ -444,7 +441,6 @@ static void filter_free_subsystem_preds(struct event_subsystem *system) remove_filter_string(call->filter); } } - mutex_unlock(&event_mutex); } static int filter_add_pred_fn(struct filter_parse_state *ps, @@ -631,7 +627,6 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps, filter->preds[filter->n_preds] = pred; filter->n_preds++; - mutex_lock(&event_mutex); list_for_each_entry(call, &ftrace_events, list) { if (!call->define_fields) @@ -642,14 +637,12 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps, err = filter_add_pred(ps, call, pred); if (err) { - mutex_unlock(&event_mutex); filter_free_subsystem_preds(system); parse_error(ps, FILT_ERR_BAD_SUBSYS_FILTER, 0); goto out; } replace_filter_string(call->filter, filter_string); } - mutex_unlock(&event_mutex); out: return err; } @@ -1076,12 +1069,12 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string) struct filter_parse_state *ps; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (!strcmp(strstrip(filter_string), "0")) { filter_disable_preds(call); remove_filter_string(call->filter); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return 0; } @@ -1109,7 +1102,7 @@ out: postfix_clear(ps); kfree(ps); out_unlock: - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return err; } @@ -1121,12 +1114,12 @@ int apply_subsystem_event_filter(struct event_subsystem *system, struct filter_parse_state *ps; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (!strcmp(strstrip(filter_string), "0")) { filter_free_subsystem_preds(system); remove_filter_string(system->filter); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return 0; } @@ -1154,7 +1147,7 @@ out: postfix_clear(ps); kfree(ps); out_unlock: - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return err; } -- 1.5.4.rc3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:tracing/urgent] tracing/filters: fix race between filter setting and module unload 2009-06-16 8:39 ` [PATCH 2/2] tracing/filters: fix race between filter setting and module unload Li Zefan @ 2009-06-17 11:13 ` tip-bot for Li Zefan 0 siblings, 0 replies; 4+ messages in thread From: tip-bot for Li Zefan @ 2009-06-17 11:13 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, rostedt, lizf, tglx Commit-ID: 00e95830a4d6e49f764fdb19896a89199bc0aa3b Gitweb: http://git.kernel.org/tip/00e95830a4d6e49f764fdb19896a89199bc0aa3b Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Tue, 16 Jun 2009 16:39:41 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Tue, 16 Jun 2009 16:25:37 -0400 tracing/filters: fix race between filter setting and module unload Module unload is protected by event_mutex, while setting filter is protected by filter_mutex. This leads to the race: echo 'bar == 0 || bar == 10' \ | > sample/filter | | insmod sample.ko add_pred("bar == 0") | -> n_preds == 1 | add_pred("bar == 100") | -> n_preds == 2 | | rmmod sample.ko | insmod sample.ko add_pred("&&") | -> n_preds == 1 (should be 3) | Now event->filter->preds is corrupted. An then when filter_match_preds() is called, the WARN_ON() in it will be triggered. To avoid the race, we remove filter_mutex, and replace it with event_mutex. [ Impact: prevent corruption of filters by module removing and loading ] Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4A375A4D.6000205@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/trace_events_filter.c | 27 ++++++++++----------------- 1 files changed, 10 insertions(+), 17 deletions(-) diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index d9f01c1..936c621 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c @@ -27,8 +27,6 @@ #include "trace.h" #include "trace_output.h" -static DEFINE_MUTEX(filter_mutex); - enum filter_op_ids { OP_OR, @@ -294,12 +292,12 @@ void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s) { struct event_filter *filter = call->filter; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (filter->filter_string) trace_seq_printf(s, "%s\n", filter->filter_string); else trace_seq_printf(s, "none\n"); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); } void print_subsystem_event_filter(struct event_subsystem *system, @@ -307,12 +305,12 @@ void print_subsystem_event_filter(struct event_subsystem *system, { struct event_filter *filter = system->filter; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (filter->filter_string) trace_seq_printf(s, "%s\n", filter->filter_string); else trace_seq_printf(s, "none\n"); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); } static struct ftrace_event_field * @@ -434,7 +432,6 @@ static void filter_free_subsystem_preds(struct event_subsystem *system) filter->n_preds = 0; } - mutex_lock(&event_mutex); list_for_each_entry(call, &ftrace_events, list) { if (!call->define_fields) continue; @@ -444,7 +441,6 @@ static void filter_free_subsystem_preds(struct event_subsystem *system) remove_filter_string(call->filter); } } - mutex_unlock(&event_mutex); } static int filter_add_pred_fn(struct filter_parse_state *ps, @@ -631,7 +627,6 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps, filter->preds[filter->n_preds] = pred; filter->n_preds++; - mutex_lock(&event_mutex); list_for_each_entry(call, &ftrace_events, list) { if (!call->define_fields) @@ -642,14 +637,12 @@ static int filter_add_subsystem_pred(struct filter_parse_state *ps, err = filter_add_pred(ps, call, pred); if (err) { - mutex_unlock(&event_mutex); filter_free_subsystem_preds(system); parse_error(ps, FILT_ERR_BAD_SUBSYS_FILTER, 0); goto out; } replace_filter_string(call->filter, filter_string); } - mutex_unlock(&event_mutex); out: return err; } @@ -1076,12 +1069,12 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string) struct filter_parse_state *ps; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (!strcmp(strstrip(filter_string), "0")) { filter_disable_preds(call); remove_filter_string(call->filter); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return 0; } @@ -1109,7 +1102,7 @@ out: postfix_clear(ps); kfree(ps); out_unlock: - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return err; } @@ -1121,12 +1114,12 @@ int apply_subsystem_event_filter(struct event_subsystem *system, struct filter_parse_state *ps; - mutex_lock(&filter_mutex); + mutex_lock(&event_mutex); if (!strcmp(strstrip(filter_string), "0")) { filter_free_subsystem_preds(system); remove_filter_string(system->filter); - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return 0; } @@ -1154,7 +1147,7 @@ out: postfix_clear(ps); kfree(ps); out_unlock: - mutex_unlock(&filter_mutex); + mutex_unlock(&event_mutex); return err; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip:tracing/urgent] tracing/filters: free filter_string in destroy_preds() 2009-06-16 8:39 [PATCH 1/2] tracing/filters: free filter_string in destroy_preds() Li Zefan 2009-06-16 8:39 ` [PATCH 2/2] tracing/filters: fix race between filter setting and module unload Li Zefan @ 2009-06-17 11:13 ` tip-bot for Li Zefan 1 sibling, 0 replies; 4+ messages in thread From: tip-bot for Li Zefan @ 2009-06-17 11:13 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, rostedt, lizf, tglx Commit-ID: 57be88878e7aa38750384704d811485a607bbda4 Gitweb: http://git.kernel.org/tip/57be88878e7aa38750384704d811485a607bbda4 Author: Li Zefan <lizf@cn.fujitsu.com> AuthorDate: Tue, 16 Jun 2009 16:39:12 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Tue, 16 Jun 2009 16:25:35 -0400 tracing/filters: free filter_string in destroy_preds() filter->filter_string is not freed when unloading a module: # insmod trace-events-sample.ko # echo "bar < 100" > /mnt/tracing/events/sample/foo_bar/filter # rmmod trace-events-sample.ko [ Impact: fix memory leak when unloading module ] Signed-off-by: Li Zefan <lizf@cn.fujitsu.com> LKML-Reference: <4A375A30.9060802@cn.fujitsu.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/trace_events_filter.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c index b24ab0e..d9f01c1 100644 --- a/kernel/trace/trace_events_filter.c +++ b/kernel/trace/trace_events_filter.c @@ -381,6 +381,7 @@ void destroy_preds(struct ftrace_event_call *call) filter_free_pred(filter->preds[i]); } kfree(filter->preds); + kfree(filter->filter_string); kfree(filter); call->filter = NULL; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-06-17 11:14 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-06-16 8:39 [PATCH 1/2] tracing/filters: free filter_string in destroy_preds() Li Zefan 2009-06-16 8:39 ` [PATCH 2/2] tracing/filters: fix race between filter setting and module unload Li Zefan 2009-06-17 11:13 ` [tip:tracing/urgent] " tip-bot for Li Zefan 2009-06-17 11:13 ` [tip:tracing/urgent] tracing/filters: free filter_string in destroy_preds() tip-bot for Li Zefan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.