[PATCH] tracing: Reject filter expressions that end on an operator

[JongHyuk Kim (MalHyuk)]

predicate_parse() seeds every new program entry with a backwards link:

        prog[N].target = N-1;

so prog[0].target starts out as -1. It is expected to be overwritten by
update_preds() before the second pass walks the program.

If the expression ends right after "&&" or "||", the pending PROCESS_AND
or PROCESS_OR bit on the operator stack is never consumed, update_preds()
never runs, and prog[0].target is still -1 when the second pass starts:

        for (i = N-1 ; i--; ) {
                int target = prog[i].target;
                if (prog[i].when_to_branch == prog[target].when_to_branch)
                        prog[i].target = prog[target].target;
        }

That reads prog[-1], 16 bytes in front of the kcalloc()ed program. The
value read there is then stored back as a branch target, and the only
check on a branch target is the lower bound in the third pass:

        if (WARN_ON(prog[i].target <= i))

so any positive value is accepted, and filter_match_preds() goes on to
use it as an unchecked index into prog[].

Reproduced on 6.12.112 with KASAN as uid 1000, with perf_event_paranoid=2
and without tracefs. One perf_event_open() and one ioctl() are enough:

        a.type = PERF_TYPE_TRACEPOINT;
        a.config = <any tracepoint id>;
        a.exclude_kernel = 1;
        fd = perf_event_open(&a, 0, -1, -1, 0);
        ioctl(fd, PERF_EVENT_IOC_SET_FILTER, "common_pid==1||common_pid==1&&");

        BUG: KASAN: slab-out-of-bounds in process_preds+0x151e/0x1b60
        Read of size 4 at addr ffff8881014ad474 by task exp_progm1/123
        CPU: 1 UID: 1000 PID: 123 Comm: exp_progm1 Not tainted 6.12.112-dirty #1
        Call Trace:
         process_preds
         create_filter
         ftrace_profile_set_filter
         _perf_ioctl
         perf_ioctl
         __x64_sys_ioctl

The faulting address is prog - 12, that is prog[-1].when_to_branch. On a
kernel built with CONFIG_SLAB_VIRTUAL the same read lands on the unmapped
guard page in front of the slab whenever the program happens to be the
first object in it, and that is a fatal page fault rather than a silent
read.

An expression is not supposed to end on an operator, and that is the only
way a PROCESS_AND or PROCESS_OR bit survives to the end of the parse, so
reject it there.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Cc: sta...@vger.kernel.org
Signed-off-by: JongHyuk Kim (MalHyuk) <malhyu...@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
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -602,6 +602,13 @@
                goto out_free;
        }
 
+       if (*top & (PROCESS_AND | PROCESS_OR)) {
+               /* Expression ends on an operator */
+               ret = -EINVAL;
+               parse_error(pe, FILT_ERR_INVALID_OP, ptr - str);
+               goto out_free;
+       }
+
        if (!N) {
                /* No program? */
                ret = -EINVAL;