Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] tracing: unreachable operator check in predicate_parse() --
commit 6ada36f117f4b8f90eaf23b50ed6bf590c01cc94 Author: JongHyuk Kim (MalHyuk) <[email protected]> tracing: Reject filter expressions that end on an operator This commit adds a check in predicate_parse() to reject event filter expressions where a pending PROCESS_AND or PROCESS_OR flag remains on the operator stack after parsing completes. > diff --git a/kernel/trace/trace_events_filter.c > b/kernel/trace/trace_events_filter.c > index 5e74d2dae40a3..744e70668940f 100644 > --- a/kernel/trace/trace_events_filter.c > +++ b/kernel/trace/trace_events_filter.c > @@ -611,6 +611,13 @@ predicate_parse(const char *str, int nr_parens, int > nr_preds, > 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; > + } [Severity: Medium] Can this check ever be reached with PROCESS_AND or PROCESS_OR set in *top? In kernel/trace/trace_events_filter.c:predicate_parse(), PROCESS_AND and PROCESS_OR are only set inside the inner loop after at least one predicate has been parsed (N >= 1), and setting either flag immediately breaks out to the outer while (*ptr) loop: if (*top & PROCESS_AND) { update_preds(prog, N - 1, invert); *top &= ~PROCESS_AND; } if (*next == '&') { *top |= PROCESS_AND; break; } if (*top & PROCESS_OR) { update_preds(prog, N - 1, !invert); *top &= ~PROCESS_OR; } if (*next == '|') { *top |= PROCESS_OR; break; } if (!*next) goto out; If an expression ends on "&&" or "||" (such as "common_pid==1||common_pid==1&&"), the outer while (*ptr) loop terminates and hits the check added by commit 1fdc4164e488 ("tracing: Reject incomplete event filter expressions") right above the out label: /* Only the inner loop can complete a non-empty expression. */ if (N) { ret = -EINVAL; parse_error(pe, FILT_ERR_INVALID_FILTER, ptr - str); goto out_free; } out: Because N >= 1 whenever PROCESS_AND or PROCESS_OR is set, falling out of the outer loop always takes that goto out_free path before reaching out. The only other way to reach out is via if (!*next) goto out inside the inner loop, which only runs after both PROCESS_AND and PROCESS_OR have just been cleared from *top in the same iteration. Doesn't that make this new check (and FILT_ERR_INVALID_OP) unreachable dead code? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
