[PATCH] tracing: Reject a filter expression that ends on a dangling operator

[PATCH] tracing: Reject a filter expression that ends on a dangling operator

[JongHyuk Kim(MalHyuk)]

predicate_parse() seeds every program entry with prog[N].target = N-1, so
prog[0].target starts at -1. That -1 is meant to be replaced before the
second pass runs, either by update_preds() when a pending PROCESS_AND or
PROCESS_OR is retired, or by the prog[N-1].target = N assignment that
fixes up the last entry.

A filter string that ends on an operator reaches the second pass with
neither having happened. The pending PROCESS_OR of "a || b &&" is only
retired after the "*next == '&'" early break, so update_preds() is never
called for it, and the trailing operator is not itself an error: the
inner loop breaks, the outer loop sees the NUL and falls through to out.
With N == 2 the prog[N-1] assignment repairs index 1, and prog[0].target
is still -1 when the second pass dereferences prog[target].

Reproducing it needs no tracefs mount and no privilege, because the
filter can be set through PERF_EVENT_IOC_SET_FILTER on a tracepoint
event, and neither ftrace_profile_set_filter() nor create_filter()
checks any capability. An ordinary user can open such an event at the
default perf_event_paranoid of 2 as long as exclude_kernel is set:

	struct perf_event_attr a = {
		.type = PERF_TYPE_TRACEPOINT, .size = sizeof(a),
		.config = <id of any syscalls:sys_enter_*>,
		.disabled = 1, .exclude_kernel = 1, .exclude_hv = 1,
	};
	int fd = perf_event_open(&a, 0, -1, -1, 0);
	ioctl(fd, PERF_EVENT_IOC_SET_FILTER,
	      "common_pid==1||common_pid==1&&");

On v6.12.112 built with KASAN this gives, within a few seconds of boot,
from uid 1000:

  BUG: KASAN: slab-out-of-bounds in process_preds+0x151e/0x1b60
  Read of size 4 at addr ffff888102657c74 by task poc/123
  CPU: 0 UID: 1000 PID: 123 Comm: poc Not tainted 6.12.112 #1
  Call Trace:
   process_preds+0x151e/0x1b60
   create_filter+0x113/0x1e0
   ftrace_profile_set_filter+0x10c/0x6b0
   _perf_ioctl+0x992/0x1830
   perf_ioctl+0x60/0x90
   __x64_sys_ioctl+0x134/0x1a0

Four boots, four reports, 2.7 s to 11.4 s after boot.

A dangling operator always leaves its PROCESS_AND or PROCESS_OR bit set
on the operator stack, so testing for that at out covers every form of
the expression, including the ones where the unresolved entry is not
index 0, such as "a && b || c &&".

Note that this changes the result for three shapes that are accepted
today without reading out of bounds, because the entry they leave
unresolved happens to be repaired by the prog[N-1] assignment: "a ||",
"a &&" and "a && b ||" now return -EINVAL with "Meaningless filter
expression" instead of installing a filter. They have no right-hand side
to evaluate, so rejecting them uniformly seemed better than resolving
the pending operator at out and giving a trailing operator a meaning.

I verified this by building the same config with and without the patch
and running both under KASAN. The trigger above goes from a KASAN report
to -EINVAL and the whole boot produces no report; "a && b || c &&",
"a ||" and "a &&" also become -EINVAL; and seven well formed filters
including parenthesised and negated ones are accepted unchanged on both.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Cc: stable@vger.kernel.org
Signed-off-by: JongHyuk Kim(MalHyuk) <malhyuk97@gmail.com>
---
 kernel/trace/trace_events_filter.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 1111111..2222222 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -602,6 +602,13 @@ predicate_parse(const char *str, int nr_parens, int nr_preds,
 		goto out_free;
 	}
 
+	if (*top & (PROCESS_AND | PROCESS_OR)) {
+		/* Expression ended on an operator with no right-hand side */
+		ret = -EINVAL;
+		parse_error(pe, FILT_ERR_INVALID_FILTER, ptr - str);
+		goto out_free;
+	}
+
 	if (!N) {
 		/* No program? */
 		ret = -EINVAL;
-- 
2.43.0