[PATCH v2 5/9] tracing/filters: Fix function event operand parsing in parse_pred()

[Masami Hiramatsu (Google)]

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

In parse_pred(), the ftrace_event_is_function(call) branch has several
bugs when parsing the string operand:

1. The loop 'for (i++; str[i]; i++)' unconditionally increments 'i' before
   checking whether str[i] is the NUL terminator. When the operand is empty
   (e.g. 'ip == '), this advances 'i' past the NUL terminator and reads
   out-of-bounds memory until another zero byte is encountered.

2. When the operand is quoted (q != 0), the loop breaks with str[i] at the
   closing quote, but 'i' is never incremented past it (unlike the regular
   string literal parser which does 'i++'). Furthermore, it does not check
   for a missing closing quote (!str[i]). Returning 'i' at the closing
   quote causes predicate_parse() to immediately re-invoke parse_pred() on
   the closing quote and fail with FILT_ERR_TOO_MANY_PREDS.

3. If the operand length is 0 (e.g. 'ip == )' or 'ip == ""'), it accepts an
   empty pattern instead of rejecting it with FILT_ERR_INVALID_VALUE.

4. When the operand is unquoted (q == 0), stray quotes or '(' inside the
   operand are consumed as part of the pattern, desynchronizing
   parse_pred() from calc_stack().

Fix this by advancing 'i' and 's' past the opening quote before the loop,
starting the loop at the current 'i' without a blind increment, checking
for missing closing quotes, advancing 'i' past the closing quote, and
rejecting empty operands or stray quotes/'(' in unquoted operands.

Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
---
 kernel/trace/trace_events_filter.c |   37 ++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index cc3a53e7cdbd..3bd0b47a627f 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1834,22 +1834,39 @@ static int parse_pred(const char *str, void *data,
 		 * Quotes are not required, but if they exist then we need
 		 * to read them till we hit a matching one.
 		 */
-		if (str[i] == '\'' || str[i] == '"')
-			q = str[i];
-		else
+		if (str[i] == '\'' || str[i] == '"') {
+			q = str[i++];
+			s = i;
+		} else {
 			q = 0;
+		}
 
-		for (i++; str[i]; i++) {
+		for (; str[i]; i++) {
 			if (q && str[i] == q)
 				break;
-			if (!q && (str[i] == ')' || str[i] == '&' ||
-				   str[i] == '|'))
-				break;
+			if (!q) {
+				if (str[i] == ')' || str[i] == '&' ||
+				    str[i] == '|')
+					break;
+				if (str[i] == '(' || str[i] == '\'' ||
+				    str[i] == '"') {
+					parse_error(pe, FILT_ERR_INVALID_VALUE,
+						    pos + i);
+					goto err_free;
+				}
+			}
+		}
+		if (q && !str[i]) {
+			parse_error(pe, FILT_ERR_MISSING_QUOTE, pos + i);
+			goto err_free;
 		}
-		/* Skip quotes */
-		if (q)
-			s++;
 		len = i - s;
+		if (q)
+			i++;
+		if (!len) {
+			parse_error(pe, FILT_ERR_INVALID_VALUE, pos + i);
+			goto err_free;
+		}
 		if (len >= MAX_FILTER_STR_VAL) {
 			parse_error(pe, FILT_ERR_OPERAND_TOO_LONG, pos + i);
 			goto err_free;