[PATCH] tracing/hist: bound expression string construction

[Pengpeng Hou]

expr_str() allocates a fixed MAX_FILTER_STR_VAL buffer and then builds
expression names with a series of raw strcat() appends. Nested operands,
constants and field flags can push the rendered string past that fixed
limit before the name is attached to the hist field.

Convert the construction helpers to explicit bounded appends and
propagate failures back to the expression parser when the rendered name
would exceed MAX_FILTER_STR_VAL.

Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
---
 kernel/trace/trace_events_hist.c | 101 +++++++++++++++++++++++--------
 1 file changed, 76 insertions(+), 25 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 73ea180cad55..caaa262360d2 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1738,85 +1738,121 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
 	return flags_str;
 }
 
-static void expr_field_str(struct hist_field *field, char *expr)
+static bool expr_append(char *expr, size_t *len, const char *str)
 {
-	if (field->flags & HIST_FIELD_FL_VAR_REF)
-		strcat(expr, "$");
-	else if (field->flags & HIST_FIELD_FL_CONST) {
+	size_t str_len = strlen(str);
+
+	if (*len + str_len >= MAX_FILTER_STR_VAL)
+		return false;
+
+	memcpy(expr + *len, str, str_len + 1);
+	*len += str_len;
+	return true;
+}
+
+static bool expr_field_str(struct hist_field *field, char *expr, size_t *len)
+{
+	if (field->flags & HIST_FIELD_FL_VAR_REF) {
+		if (!expr_append(expr, len, "$"))
+			return false;
+	} else if (field->flags & HIST_FIELD_FL_CONST) {
 		char str[HIST_CONST_DIGITS_MAX];
+		int ret;
+
+		ret = snprintf(str, sizeof(str), "%llu", field->constant);
+		if (ret >= sizeof(str))
+			return false;
 
-		snprintf(str, HIST_CONST_DIGITS_MAX, "%llu", field->constant);
-		strcat(expr, str);
+		if (!expr_append(expr, len, str))
+			return false;
 	}
 
-	strcat(expr, hist_field_name(field, 0));
+	if (!expr_append(expr, len, hist_field_name(field, 0)))
+		return false;
 
 	if (field->flags && !(field->flags & HIST_FIELD_FL_VAR_REF)) {
 		const char *flags_str = get_hist_field_flags(field);
 
 		if (flags_str) {
-			strcat(expr, ".");
-			strcat(expr, flags_str);
+			if (!expr_append(expr, len, ".") ||
+			    !expr_append(expr, len, flags_str))
+				return false;
 		}
 	}
+
+	return true;
 }
 
 static char *expr_str(struct hist_field *field, unsigned int level)
 {
 	char *expr;
+	size_t len = 0;
 
 	if (level > 1)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	expr = kzalloc(MAX_FILTER_STR_VAL, GFP_KERNEL);
 	if (!expr)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
 	if (!field->operands[0]) {
-		expr_field_str(field, expr);
+		if (!expr_field_str(field, expr, &len))
+			goto free;
 		return expr;
 	}
 
 	if (field->operator == FIELD_OP_UNARY_MINUS) {
 		char *subexpr;
 
-		strcat(expr, "-(");
+		if (!expr_append(expr, &len, "-("))
+			goto free;
 		subexpr = expr_str(field->operands[0], ++level);
 		if (!subexpr) {
-			kfree(expr);
-			return NULL;
+			goto free;
+		}
+		if (!expr_append(expr, &len, subexpr) ||
+		    !expr_append(expr, &len, ")")) {
+			kfree(subexpr);
+			goto free;
 		}
-		strcat(expr, subexpr);
-		strcat(expr, ")");
 
 		kfree(subexpr);
 
 		return expr;
 	}
 
-	expr_field_str(field->operands[0], expr);
+	if (!expr_field_str(field->operands[0], expr, &len))
+		goto free;
 
 	switch (field->operator) {
 	case FIELD_OP_MINUS:
-		strcat(expr, "-");
+		if (!expr_append(expr, &len, "-"))
+			goto free;
 		break;
 	case FIELD_OP_PLUS:
-		strcat(expr, "+");
+		if (!expr_append(expr, &len, "+"))
+			goto free;
 		break;
 	case FIELD_OP_DIV:
-		strcat(expr, "/");
+		if (!expr_append(expr, &len, "/"))
+			goto free;
 		break;
 	case FIELD_OP_MULT:
-		strcat(expr, "*");
+		if (!expr_append(expr, &len, "*"))
+			goto free;
 		break;
 	default:
-		kfree(expr);
-		return NULL;
+		goto free;
 	}
 
-	expr_field_str(field->operands[1], expr);
+	if (!expr_field_str(field->operands[1], expr, &len))
+		goto free;
 
 	return expr;
+
+free:
+	kfree(expr);
+	return ERR_PTR(-E2BIG);
 }
 
 /*
@@ -2630,6 +2666,11 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
 	expr->is_signed = operand1->is_signed;
 	expr->operator = FIELD_OP_UNARY_MINUS;
 	expr->name = expr_str(expr, 0);
+	if (IS_ERR(expr->name)) {
+		ret = PTR_ERR(expr->name);
+		expr->name = NULL;
+		goto free;
+	}
 	expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
 	if (!expr->type) {
 		ret = -ENOMEM;
@@ -2842,6 +2883,11 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		destroy_hist_field(operand1, 0);
 
 		expr->name = expr_str(expr, 0);
+		if (IS_ERR(expr->name)) {
+			ret = PTR_ERR(expr->name);
+			expr->name = NULL;
+			goto free_expr;
+		}
 	} else {
 		/* The operand sizes should be the same, so just pick one */
 		expr->size = operand1->size;
@@ -2855,6 +2901,11 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		}
 
 		expr->name = expr_str(expr, 0);
+		if (IS_ERR(expr->name)) {
+			ret = PTR_ERR(expr->name);
+			expr->name = NULL;
+			goto free_expr;
+		}
 	}
 
 	return expr;
-- 
2.50.1 (Apple Git-155)