Re: [PATCH v3 11/16] tracing/filters: Check for trailing characters after newline in filter string

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing/filters: in-place truncation on error in filter_check_newline()
--

commit 8f43e3c8d713c8d0669d239d3455b033bcd6f4f2
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

tracing/filters: Check for trailing characters after newline in filter string

This commit introduces filter_check_newline() to truncate the filter string
at the first newline and reject inputs that contain non-whitespace
characters after that newline with -EINVAL.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 9d1446464058a..f3c18324b07a8 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -2441,6 +2441,31 @@ static int process_system_preds(struct trace_subsystem_dir *dir,
>  	return -ENOMEM;
>  }
>  
> +/*
> + * Convert the first newline to '\0' and check that the rest of the input
> + * contains no non-whitespace characters.
> + */
> +static int filter_check_newline(char *str)
> +{
> +	char *p;
> +
> +	if (!str)
> +		return 0;
> +
> +	p = strchr(str, '\n');
> +	if (!p)
> +		return 0;
> +
> +	*p++ = '\0';
> +	while (*p) {
> +		if (!isspace(*p))
> +			return -EINVAL;
> +		p++;
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Should filter_check_newline() verify that all trailing characters are
whitespace before writing '\0' to str?

Because *p++ = '\0' mutates str in-place before the loop checks the remaining
characters, returning -EINVAL leaves str truncated at the newline.

When __ftrace_set_clr_event_nolock() in kernel/trace/trace_events.c iterates
over matching events and passes the same filter buffer to apply_event_filter():

kernel/trace/trace_events.c:__ftrace_set_clr_event_nolock() {
	list_for_each_entry(file, &tr->events, list) {
		...
		if (filter)
			ret = apply_event_filter(file, filter);

		if (!filter || !ret)
			ret = ftrace_event_enable_disable(file, set);
	}
}

a multi-line filter string with non-whitespace characters after the newline
fails with -EINVAL on the first matching event, but truncates filter in-place.
On subsequent matching events in the loop, filter_check_newline() no longer
sees the newline and returns 0, causing the truncated filter to be applied and
those events to be enabled.

Similarly, in trace_boot_init_one_event() in kernel/trace/trace_boot.c:

		else if (apply_event_filter(file, buf) < 0)
			pr_err("Failed to apply filter: %s\n", buf);

the error message logs the truncated prefix in buf instead of the full
invalid filter string.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179314820431.684037.4147763044424284198.stgit@devnote2?part=11