Re: [PATCH v2 8/9] tracing/filters: Terminate filter string at first newline

[Masami Hiramatsu (Google)]

On Mon, 26 Oct 2026 16:49:57 -0400, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Sun, 25 Oct 2026 17:38:22 +0900
> "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > 
> > Each trace event or subsystem only holds a single filter, and
> > append_filter_err() / tracing_log_err() caret placement assumes a
> > single-line filter string. However, apply_event_filter() and
> > apply_subsystem_event_filter() only call strstrip(), which strips
> > trailing newlines but leaves embedded newlines intact.
> > 
> > An embedded newline in a filter string (for example inside 'CPUS{0\n"}')
> > causes cpulist_parse() / bitmap_parselist() to treat '\n' as the end of
> > the cpulist and ignore trailing characters before '}', desynchronizing
> > quote tracking between calc_stack() and parse_pred() and misaligning
> > error_log caret reporting.
> > 
> > Terminate filter_string at the first '\n' in apply_event_filter(),
> > apply_subsystem_event_filter(), and create_filter_start() so only the
> > first line is parsed.
> 
> How about also checking if there's any non white space after the newline,
> and if so, fail the filter outright with -EINVAL?

Hm, indeed. That sounds good to me, because user gave a wrong input.

Thanks!

> 
> -- Steve
> 
> 
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > ---
> >  kernel/trace/trace_events_filter.c |   15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 8d7fb6e9d7f8..14afcca7a444 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -2426,10 +2426,15 @@ static int create_filter_start(char *filter_string, bool set_str,
> >  	struct event_filter *filter;
> >  	struct filter_parse_error *pe = NULL;
> >  	int err = 0;
> > +	char *p;
> >  
> >  	if (WARN_ON_ONCE(*pse || *filterp))
> >  		return -EINVAL;
> >  
> > +	p = strchr(filter_string, '\n');
> > +	if (p)
> > +		*p = '\0';
> > +
> >  	filter = kzalloc_obj(*filter);
> >  	if (filter && set_str) {
> >  		filter->filter_string = kstrdup(filter_string, GFP_KERNEL);
> > @@ -2543,11 +2548,16 @@ int apply_event_filter(struct trace_event_file *file, char *filter_string)
> >  {
> >  	struct trace_event_call *call = file->event_call;
> >  	struct event_filter *filter = NULL;
> > +	char *p;
> >  	int err;
> >  
> >  	if (file->flags & EVENT_FILE_FL_FREED)
> >  		return -ENODEV;
> >  
> > +	p = strchr(filter_string, '\n');
> > +	if (p)
> > +		*p = '\0';
> > +
> >  	if (!strcmp(strstrip(filter_string), "0")) {
> >  		filter_disable(file);
> >  		filter = event_filter(file);
> > @@ -2595,6 +2605,7 @@ int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
> >  	struct trace_array *tr = dir->tr;
> >  	struct event_filter *filter = NULL;
> >  	int err = 0;
> > +	char *p;
> >  
> >  	guard(mutex)(&event_mutex);
> >  
> > @@ -2602,6 +2613,10 @@ int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
> >  	if (!dir->nr_events)
> >  		return -ENODEV;
> >  
> > +	p = strchr(filter_string, '\n');
> > +	if (p)
> > +		*p = '\0';
> > +
> >  	if (!strcmp(strstrip(filter_string), "0")) {
> >  		filter_free_subsystem_preds(dir, tr);
> >  		remove_filter_string(system->filter);
> 
> 
>

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>