From: Masami Hiramatsu (Google) <[email protected]>
In process_system_preds(), "fail" was initialized to true to indicate
whether any event in the subsystem successfully applied the filter.
However, "fail = false;" was placed at the end of the event loop outside
the error check, unconditionally clearing "fail" on every iteration even
when process_preds() returned an error.
As a result, if every event in the subsystem fails to match the filter
(e.g., an invalid field), process_system_preds() returns 0 (success)
instead of -EINVAL. Consequently, create_system_filter() considers the
operation successful and silently resets the subsystem filter display to
the default global filter message instead of reporting the parse error.
Replace "fail" with "ret", clearing it to 0 only when at least one event
filter succeeds. If "ret" is still non-zero after the loop, jump to
"fail" to report FILT_ERR_BAD_SUBSYS_FILTER and fall through to
"fail_mem". In "fail_mem", free any replaced filters via
delay_free_filter() if !list_empty(&filter_list->list), or via
free_filter_list() otherwise, consolidating the cleanup paths.
Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Cc: [email protected]
Suggested-by: Steven Rostedt <[email protected]>
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
Changes in v4:
- Clean up process_system_preds() by replacing the 'fail' boolean with
'ret' and consolidating the cleanup logic between fail and fail_mem,
as suggested by Steven Rostedt.
---
kernel/trace/trace_events_filter.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c
b/kernel/trace/trace_events_filter.c
index 28977d10e60b..b35ecb01697b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2337,7 +2337,7 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
struct filter_list *filter_item;
struct event_filter *filter = NULL;
struct filter_head *filter_list;
- bool fail = true;
+ int ret = -ENOMEM;
int err;
filter_list = kmalloc_obj(*filter_list);
@@ -2364,8 +2364,10 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
filter_disable(file);
parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
append_filter_err(tr, pe, filter);
- } else
+ } else {
event_set_filtered_flag(file);
+ ret = 0;
+ }
filter_item = kzalloc_obj(*filter_item);
@@ -2380,11 +2382,9 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
filter_item->filter = event_filter(file);
event_set_filter(file, filter);
filter = NULL;
-
- fail = false;
}
- if (fail)
+ if (ret)
goto fail;
/*
@@ -2396,19 +2396,18 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
return 0;
fail:
/* No call succeeded */
- free_filter_list(filter_list);
parse_error(pe, FILT_ERR_BAD_SUBSYS_FILTER, 0);
- return -EINVAL;
+ ret = -EINVAL;
fail_mem:
__free_filter(filter);
- /* If any call succeeded, we still need to sync */
- if (!fail)
+ /* If any filter was replaced, we still need to sync */
+ if (!list_empty(&filter_list->list))
delay_free_filter(filter_list);
else
free_filter_list(filter_list);
- return -ENOMEM;
+ return ret;
}
static int create_filter_start(char *filter_string, bool set_str,
