From: Masami Hiramatsu (Google) <[email protected]>
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: [email protected]
Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
---
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;
