From: Masami Hiramatsu (Google) <[email protected]>

In predicate_parse(), 'ptr' has already been advanced past 'next',
shifting 'pos' passed to parse_pred() one character to the right.

In parse_pred(), several error paths report 'pos + i' after 'i' has
already been advanced to subsequent tokens:
- When a field is not found (FILT_ERR_FIELD_NOT_FOUND) or when a
  function event field is not "ip" (FILT_ERR_IP_FIELD_ONLY), 'pos + i'
  points past the field name or to the operand instead of the field.
- In operator validation checks for string and numeric operands, and
  when select_comparison_fn() returns NOP, 'pos + i' points to the
  operand or past it instead of the unsupported operator.

Fix these by passing 'next - str' in predicate_parse(), saving the
field position in 'field_pos' and the operator position in 'op_pos', and
passing 'pos + field_pos' or 'pos + op_pos' accordingly. Also update the
tracing-error-log.tc selftest to reflect the corrected error position.

Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
Changes in v4:
 - Drop Fixes and Cc: stable tags since this error position fix is a
   cleanup.
Changes in v3:
 - Group all error position fixes for commit 80765597bc58 together into
   a single patch for ease of backporting.
---
 kernel/trace/trace_events_filter.c                 |   19 ++++++++++++-------
 .../ftrace/test.d/ftrace/tracing-error-log.tc      |    2 +-
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c 
b/kernel/trace/trace_events_filter.c
index e91ec41bfd2a..6ff208f0e033 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -537,7 +537,7 @@ predicate_parse(const char *str, int nr_parens, int 
nr_preds,
                inverts[N] = invert;                            /* #4 */
                prog[N].target = N-1;
 
-               len = parse_pred(next, data, ptr - str, pe, &prog[N].pred);
+               len = parse_pred(next, data, next - str, pe, &prog[N].pred);
                if (len < 0) {
                        ret = len;
                        goto out_free;
@@ -1700,12 +1700,15 @@ static int parse_pred(const char *str, void *data,
        int ret;
        int op;
        int s;
+       int field_pos = 0;
+       int op_pos = 0;
        int i = 0;
 
        /* First find the field to associate to */
        while (isspace(str[i]))
                i++;
        s = i;
+       field_pos = i;
 
        while (isalnum(str[i]) || str[i] == '_')
                i++;
@@ -1726,7 +1729,7 @@ static int parse_pred(const char *str, void *data,
        field = trace_find_event_field(call, field_name);
        kfree(field_name);
        if (!field) {
-               parse_error(pe, FILT_ERR_FIELD_NOT_FOUND, pos + i);
+               parse_error(pe, FILT_ERR_FIELD_NOT_FOUND, pos + field_pos);
                return -EINVAL;
        }
 
@@ -1745,6 +1748,8 @@ static int parse_pred(const char *str, void *data,
        while (isspace(str[i]))
                i++;
 
+       op_pos = i;
+
        /* Make sure this op is supported */
        for (op = 0; ops[op]; op++) {
                /* This is why '<=' must come before '<' in ops[] */
@@ -1753,7 +1758,7 @@ static int parse_pred(const char *str, void *data,
        }
 
        if (!ops[op]) {
-               parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
+               parse_error(pe, FILT_ERR_INVALID_OP, pos + op_pos);
                goto err_free;
        }
 
@@ -1850,7 +1855,7 @@ static int parse_pred(const char *str, void *data,
                 * (perf doesn't use it) and grab everything.
                 */
                if (strcmp(field->name, "ip") != 0) {
-                       parse_error(pe, FILT_ERR_IP_FIELD_ONLY, pos + i);
+                       parse_error(pe, FILT_ERR_IP_FIELD_ONLY, pos + 
field_pos);
                        goto err_free;
                }
                pred->fn_num = FILTER_PRED_FN_NOP;
@@ -2039,7 +2044,7 @@ static int parse_pred(const char *str, void *data,
                case OP_EQ:
                        break;
                default:
-                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
+                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + 
op_pos);
                        goto err_free;
                }
 
@@ -2140,7 +2145,7 @@ static int parse_pred(const char *str, void *data,
                }
 
                if (op == OP_GLOB) {
-                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + i);
+                       parse_error(pe, FILT_ERR_ILLEGAL_FIELD_OP, pos + 
op_pos);
                        goto err_free;
                }
 
@@ -2179,7 +2184,7 @@ static int parse_pred(const char *str, void *data,
                        pred->fn_num = select_comparison_fn(pred->op, 
field->size,
                                                            field->is_signed);
                        if (pred->fn_num == FILTER_PRED_FN_NOP) {
-                               parse_error(pe, FILT_ERR_INVALID_OP, pos + i);
+                               parse_error(pe, FILT_ERR_INVALID_OP, pos + 
op_pos);
                                goto err_free;
                        }
                        if (pred->op == OP_NE)
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc 
b/tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc
index 6c190620db47..2c900873ce3c 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/tracing-error-log.tc
@@ -10,6 +10,6 @@ fail() { #msg
     exit_fail
 }
 
-ftrace_errlog_check 'event filter parse error' '((sig >= 10 && sig < 15) || 
dsig ^== 17) && comm != bash' 'events/signal/signal_generate/filter'
+ftrace_errlog_check 'event filter parse error' '((sig >= 10 && sig < 15) || 
^dsig == 17) && comm != bash' 'events/signal/signal_generate/filter'
 
 exit 0


Reply via email to