[PATCH] tracing: Reject .function on the function trace event

[PATCH] tracing: Reject .function on the function trace event

[David Carlier]

For the function event, parse_pred() handles the .function suffix
before the ftrace_event_is_function() case, so "ip.function == schedule"
builds a predicate with no regex. Setting it through perf then
dereferences the NULL pred->regex in ftrace_function_set_filter_pred().

The "ip" field of that event already takes a function name, so reject
the suffix there.

Fixes: e6745a4da964 ("tracing: Add a way to filter function addresses to function names")
Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/trace/trace_events_filter.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c10951bcfc5f..11bb751c4388 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1743,6 +1743,12 @@ static int parse_pred(const char *str, void *data,
 	pred->op = op;
 
 	if (function) {
+		/* Function events already take a function name on 'ip' */
+		if (ftrace_event_is_function(call)) {
+			parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
+			goto err_free;
+		}
+
 		/* The field must be the same size as long */
 		if (field->size != sizeof(long)) {
 			parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
-- 
2.55.0

Re: [PATCH] tracing: Reject .function on the function trace event

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] tracing: out-of-bounds read in parse_pred() on empty
  function filter operand [1]
--

commit e67c952556cf8df0fc74a91895fdbea1c0dd54b5
Author: David Carlier <devnexen@gmail.com>

tracing: Reject .function on the function trace event

This commit rejects the .function filter suffix on the function trace
event so that parse_pred() does not build a predicate with a NULL regex.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5e74d2dae40a3..9c807191166ba 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1744,6 +1744,12 @@ static int parse_pred(const char *str, void *data,
>  	pred->op = op;
>  
>  	if (function) {
> +		/* Function events already take a function name on 'ip' */
> +		if (ftrace_event_is_function(call)) {
> +			parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
> +			goto err_free;
> +		}
> +
>  		/* The field must be the same size as long */
>  		if (field->size != sizeof(long)) {
>  			parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);

[Severity: Critical]
This problem wasn't introduced by this patch, but can the paired
ftrace_event_is_function(call) branch in parse_pred() read past the end
of str [1] when the filter has no operand after the operator (for
example, "ip ==")?

kernel/trace/trace_events_filter.c:parse_pred() {
	...
	} else if (ftrace_event_is_function(call)) {
		...
		if (str[i] == '\'' || str[i] == '"')
			q = str[i];
		else
			q = 0;

		for (i++; str[i]; i++) {
	...
}

When str ends after the operator and optional whitespace, str[i] is the
terminating NUL byte and q is set to 0.  The loop initializer i++ then
advances i one byte past the NUL terminator before checking str[i], so
parse_pred() scans out-of-bounds slab memory, copies it into
pred->regex->pattern, and returns the out-of-bounds offset to
predicate_parse().

[1] https://sashiko.dev/#/bug/linux-911099f1-cea5-4f84-ba41-25fbcbb66dad

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114195914.2678414-1-devnexen@gmail.com?part=1