[PATCH v2 0/9] tracing/filters: Fix event filter parsing and matching bugs

[Masami Hiramatsu (Google)]

Hi,

Here is the second version of fixes for various bugs in the trace event
filter parser and predicate evaluation in kernel/trace/trace_events_filter.c,
along with a new ftracetest test case covering filter syntax checks.

The previous version is here:

 https://lore.kernel.org/all/179285912318.466249.7425922682086130057.stgit@devnote2

Changes in v2:
[01/09]
 - Guard against negative length and avoid calling strnlen() when the
   buffer capacity is smaller than the search pattern.
 - Use strnlen(str, len) directly instead of calling strlen(str) when
   len is 0, avoiding an out-of-bounds read on 0-length dynamic arrays.
[03/10] (in v1)
 - Dropped because out-of-range CPU numbers are already rejected at
   filter parse time by cpulist_parse() checking against nr_cpu_ids,
   making the runtime check in do_filter_cpumask_scalar() unreachable.

Summary of the patches:

- [01/09] Compute the actual string length with strnlen() in
  regex_match_end() so suffix matching ("*pattern") works on fixed-size
  string fields (such as comm) and dynamic strings.
- [02/09] Fix process_system_preds() so that "fail" is only cleared when
  at least one event in the subsystem succeeds, and use
  delay_free_filter() when freeing replaced filters on error paths.
- [03/09] Return "pred->op == OP_NE" when "cpu >= nr_cpu_ids" in
  DEFINE_CPUMASK_COMPARISON_PRED so "!=" against a cpumask evaluates to
  true for out-of-range CPU numbers.
- [04/09] Ensure parse_error() is called on all -EINVAL error paths in
  parse_pred(), predicate_parse(), and process_preds() instead of
  leaving lasterr as 0 ("Error: (0)").
- [05/09] Fix function event ("ip") operand parsing in parse_pred() to
  avoid reading past the NUL terminator on empty operands, check for
  missing closing quotes, advance past closing quotes, and reject empty
  operands or stray quotes/'(' in unquoted operands.
- [06/09] Advance past the second character of "&&" and "||" in
  calc_stack() so the second operator character is not miscounted as a
  new predicate.
- [07/09] Reject unsupported field sizes in parse_pred() when
  select_comparison_fn() or CPUS{} size matching leaves pred->fn_num as
  FILTER_PRED_FN_NOP, avoiding a kernel warning in update_pred_fn().
- [08/09] Terminate the input filter string at the first newline ('\n')
  in create_filter_start(), apply_event_filter(), and
  apply_subsystem_event_filter() so multi-line input cannot desynchronize
  cpulist_parse()/parse_pred() from calc_stack() or misalign error_log
  caret placement. (as an improvement)
- [09/09] Add filter-syntax-check.tc to ftrace selftests to test valid
  filters, syntax error diagnostics and caret positions, multi-line
  input truncation, and subsystem filter error returns.

Notes:

- These bugs were identified and the fixes and cover letter were
prepared with the assistance of an LLM coding assistant.

- Also, note that [08/09] may need more discussion because it can break
user space tool which attempts to use line-break characters as a filter
for reasons that are not entirely clear. But as far as I can see, the
error_log does not support such multi-line filter input. So I think
we still need it. (But not sure we should backport it, maybe as an
improvement.)

Thank you,

---
base-commit: 1fdc4164e488916402badddbb1f51e9380dabb91

Masami Hiramatsu (Google) (9):
      tracing/filters: Fix MATCH_END_ONLY regex matching for fixed-size strings
      tracing/filters: Fix false success return in process_system_preds()
      tracing/filters: Handle out-of-range CPU for OP_NE in cpumask comparison
      tracing/filters: Report parse error on all -EINVAL paths in filter parsing
      tracing/filters: Fix function event operand parsing in parse_pred()
      tracing/filters: Skip second character of '&&' and '||' in calc_stack()
      tracing/filters: Reject unsupported field sizes in parse_pred()
      tracing/filters: Terminate filter string at first newline
      selftests/ftrace: Add event filter syntax check test case


 kernel/trace/trace_events_filter.c                 |  101 +++++++++++++++-----
 .../ftrace/test.d/filter/filter-syntax-check.tc    |   52 ++++++++++
 2 files changed, 130 insertions(+), 23 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc

--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>