[PATCH v4 11/15] tracing/filters: Fix error positions in filter parser

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

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) <mhiramat@xxxxxxxxxx>
---
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