* [PATCH v4.1 02/15] tracing/filters: Fix false success return in process_system_preds()
[not found] <sashiko-outbox-188613@kernel.org>
@ 2026-10-29 2:30 ` Masami Hiramatsu (Google)
0 siblings, 0 replies; only message in thread
From: Masami Hiramatsu (Google) @ 2026-10-29 2:30 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Shuah Khan
Cc: Mathieu Desnoyers, linux-kernel, linux-trace-kernel,
linux-kselftest
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
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: stable@vger.kernel.org
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
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,
^ permalink raw reply related [flat|nested] only message in thread