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.
Fix this by clearing "fail" only when at least one event filter succeeds.
Consolidate the cleanup logic in fail and fail_mem by using ret
initialized to -ENOMEM, setting ret to -EINVAL in fail, and falling
through to fail_mem, freeing any replaced filters via delay_free_filter()
if !list_empty(&filter_list->list).
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.1:
- Recover 'fail' flag instead of clearing 'ret'.
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 | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c
b/kernel/trace/trace_events_filter.c
index 28977d10e60b..981cddf01e65 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -2338,6 +2338,7 @@ static int process_system_preds(struct
trace_subsystem_dir *dir,
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,9 +2365,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);
-
+ fail = false;
+ }
filter_item = kzalloc_obj(*filter_item);
if (!filter_item)
@@ -2380,8 +2382,6 @@ 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)
@@ -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,
