[PATCH v4 00/15] tracing/filters: Fix event filter parsing and matching bugs

[Masami Hiramatsu (Google)]

Hi,

Here is the 4th 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/179314820431.684037.4147763044424284198.stgit@devnote2

In this version, according to the comment from Sashiko and Steve, dropped
a redundant patch and reorder the patches for separating real bug fixes
(1-8) and cleanups & test (9-15). Steve and I agreed that the patches which
just fixes the error message will be a cleanup or improvement, so I removed
Fixes/Cc:stable tags from those.

Changes in v4:
- Dropped the separate regex_match_end() zero-length string fix patch
  ([01/16] in v3) as redundant per review from Sashiko.
- In [01/15], introduce regex_match_end_dynstr() for FILTER_STATIC_STRING
  fields defined with size 0 (such as the 'buf' field of ftrace/print
  events / trace_marker), so that suffix filtering on unbounded NUL-terminated
  strings works as expected without penalizing fixed-size buffer matching.
- In [02/15], clean up process_system_preds() by replacing the 'fail'
  boolean with 'ret' and consolidating the cleanup logic between fail
  and fail_mem, as suggested by Steven Rostedt.
- In [09/15] (was [10/16]), verify that all characters following the
  newline are whitespace before writing '\0' in filter_check_newline(),
  preventing in-place truncation of the filter string on error, as
  pointed out by Sashiko.
- In [10/15] (was [08/16]), drop Fixes and Cc: stable tags, as this is an
  error log improvement cleanup rather than a bug fix.
  Also move it after other Cc: stable patches.
- In [11/15] to [14/15], drop Fixes and Cc: stable tags from error
  position fixes as these are cleanups.


Summary of the patches:

~~~~~~(fixes)~~~~~~
- [01/15] Introduce regex_match_end_str() for fixed-size string fields
  (FILTER_COMM, FILTER_STATIC_STRING with field->size > 0), and
  regex_match_end_dynstr() for unbounded strings (field->size == 0)
  in parse_pred().
- [02/15] 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/15] 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/15] 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/15] Check if str[i] is NUL immediately after operator whitespace
  in parse_pred() to prevent out-of-bounds reads on missing operands.
- [06/15] Fix quoted operand parsing in function event filters, check for
  missing quotes, and advance past closing quotes.
- [07/15] Reject empty unquoted operands in function event filters.
- [08/15] 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().

~~~~~~(improvement/cleanups)~~~~~~
- [09/15] Convert the first newline to NUL and reject multi-line input
  containing non-whitespace characters after the newline in
  create_filter_start(), apply_event_filter(), and
  apply_subsystem_event_filter().
- [10/15] Reject stray quotes and '(' in unquoted function event operands.
- [11/15] Fix parser error positions for base filter expressions
  (predicate_parse(), FIELD_NOT_FOUND, IP_FIELD_ONLY, string/numeric
  operators), and update tracing-error-log.tc.
- [12/15] Pass 'pos + fn_pos' for ILLEGAL_FIELD_OP and 'pos + op_pos' for
  INVALID_OP in function filter operations in parse_pred().
- [13/15] Pass 'pos + op_pos' for CPUS operator checks and check
  'str[i] != '{'' before incrementing 'i' for MISSING_BRACE_OPEN.
- [14/15] Pass 'pos + op_pos' for GLOB_WITHIN when '~' is used with a
  'within' filter.
- [15/15] Add filter-syntax-check.tc to ftrace selftests to test valid
  filters, syntax error diagnostics and caret positions, multi-line
  input validation, 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.

Thank you,

---
base-commit: 1fdc4164e488916402badddbb1f51e9380dabb91

Masami Hiramatsu (Google) (15):
      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: Check for missing operand after operator in parse_pred()
      tracing/filters: Fix quoted operand parsing in function event filter
      tracing/filters: Reject empty unquoted operand in function event filter
      tracing/filters: Reject unsupported field sizes in parse_pred()
      tracing/filters: Check for trailing characters after newline in filter string
      tracing/filters: Reject stray quotes and parentheses in unquoted function event operand
      tracing/filters: Fix error positions in filter parser
      tracing/filters: Fix error position for function filter operations
      tracing/filters: Fix error positions for CPUS mask parsing
      tracing/filters: Fix error position for within filter operator check
      selftests/ftrace: Add event filter syntax check test case


 kernel/trace/trace_events_filter.c                 |  182 ++++++++++++++++----
 .../ftrace/test.d/filter/filter-syntax-check.tc    |  100 +++++++++++
 .../ftrace/test.d/ftrace/tracing-error-log.tc      |    2 
 3 files changed, 248 insertions(+), 36 deletions(-)
 create mode 100644 tools/testing/selftests/ftrace/test.d/filter/filter-syntax-check.tc

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