expr_str() builds a histogram expression in a fixed-size
MAX_FILTER_STR_VAL allocation with unbounded strcat() calls.
Synthetic events permit field names longer than that buffer. Constructing
a trigger expression can therefore write past the allocation.
Build the expression with seq_buf. Propagate construction errors to the
parser and reject strings that overflow the fixed-size buffer with -E2BIG.
Fixes: 100719dcef44 ("tracing: Add simple expression support to hist triggers")
Cc: [email protected]
Signed-off-by: Li Qiang <[email protected]>
---
kernel/trace/trace_events_hist.c | 89 +++++++++++++++++++++-----------
1 file changed, 59 insertions(+), 30 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 82ce492ab268..2b2547546360 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1733,86 +1733,99 @@ 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 void expr_field_str(struct hist_field *field, struct seq_buf *s)
{
if (field->flags & HIST_FIELD_FL_VAR_REF) {
if (!field->system)
- strcat(expr, "$");
+ seq_buf_putc(s, '$');
} else if (field->flags & HIST_FIELD_FL_CONST) {
char str[HIST_CONST_DIGITS_MAX];
snprintf(str, HIST_CONST_DIGITS_MAX, "%llu", field->constant);
- strcat(expr, str);
+ seq_buf_puts(s, str);
}
- strcat(expr, hist_field_name(field, 0));
+ seq_buf_puts(s, hist_field_name(field, 0));
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);
+ seq_buf_putc(s, '.');
+ seq_buf_puts(s, flags_str);
}
}
}
static char *expr_str(struct hist_field *field, unsigned int level)
{
+ struct seq_buf s;
char *expr;
+ int ret = 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);
+
+ seq_buf_init(&s, expr, MAX_FILTER_STR_VAL);
if (!field->operands[0]) {
- expr_field_str(field, expr);
- return expr;
+ expr_field_str(field, &s);
+ goto out;
}
if (field->operator == FIELD_OP_UNARY_MINUS) {
char *subexpr;
- strcat(expr, "-(");
+ seq_buf_puts(&s, "-(");
subexpr = expr_str(field->operands[0], ++level);
- if (!subexpr) {
- kfree(expr);
- return NULL;
+ if (IS_ERR(subexpr)) {
+ ret = PTR_ERR(subexpr);
+ goto free;
}
- strcat(expr, subexpr);
- strcat(expr, ")");
+ seq_buf_puts(&s, subexpr);
+ seq_buf_putc(&s, ')');
kfree(subexpr);
-
- return expr;
+ goto out;
}
- expr_field_str(field->operands[0], expr);
+ expr_field_str(field->operands[0], &s);
switch (field->operator) {
case FIELD_OP_MINUS:
- strcat(expr, "-");
+ seq_buf_putc(&s, '-');
break;
case FIELD_OP_PLUS:
- strcat(expr, "+");
+ seq_buf_putc(&s, '+');
break;
case FIELD_OP_DIV:
- strcat(expr, "/");
+ seq_buf_putc(&s, '/');
break;
case FIELD_OP_MULT:
- strcat(expr, "*");
+ seq_buf_putc(&s, '*');
break;
default:
- kfree(expr);
- return NULL;
+ ret = -EINVAL;
+ goto free;
}
- expr_field_str(field->operands[1], expr);
+ expr_field_str(field->operands[1], &s);
+out:
+ seq_buf_str(&s);
+ if (seq_buf_has_overflowed(&s)) {
+ ret = -E2BIG;
+ goto free;
+ }
return expr;
+
+free:
+ kfree(expr);
+ return ERR_PTR(ret);
}
/*
@@ -2556,6 +2569,7 @@ static struct hist_field *parse_unary(struct
hist_trigger_data *hist_data,
char *var_name, unsigned int *n_subexprs)
{
struct hist_field *operand1, *expr = NULL;
+ char *expr_name;
unsigned long operand_flags;
int ret = 0;
char *s;
@@ -2625,7 +2639,12 @@ static struct hist_field *parse_unary(struct
hist_trigger_data *hist_data,
expr->size = operand1->size;
expr->is_signed = operand1->is_signed;
expr->operator = FIELD_OP_UNARY_MINUS;
- expr->name = expr_str(expr, 0);
+ expr_name = expr_str(expr, 0);
+ if (IS_ERR(expr_name)) {
+ ret = PTR_ERR(expr_name);
+ goto free;
+ }
+ expr->name = expr_name;
expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
if (!expr->type) {
ret = -ENOMEM;
@@ -2691,7 +2710,7 @@ static struct hist_field *parse_expr(struct
hist_trigger_data *hist_data,
struct hist_field *var1 = NULL, *var2 = NULL;
unsigned long operand_flags, operand2_flags;
int field_op, ret = -EINVAL;
- char *sep, *operand1_str;
+ char *expr_name, *sep, *operand1_str;
enum hist_field_fn op_fn;
bool combine_consts;
@@ -2837,7 +2856,12 @@ static struct hist_field *parse_expr(struct
hist_trigger_data *hist_data,
destroy_hist_field(operand2, 0);
destroy_hist_field(operand1, 0);
- expr->name = expr_str(expr, 0);
+ expr_name = expr_str(expr, 0);
+ if (IS_ERR(expr_name)) {
+ ret = PTR_ERR(expr_name);
+ goto free_expr;
+ }
+ expr->name = expr_name;
} else {
/* The operand sizes should be the same, so just pick one */
expr->size = operand1->size;
@@ -2850,7 +2874,12 @@ static struct hist_field *parse_expr(struct
hist_trigger_data *hist_data,
goto free_expr;
}
- expr->name = expr_str(expr, 0);
+ expr_name = expr_str(expr, 0);
+ if (IS_ERR(expr_name)) {
+ ret = PTR_ERR(expr_name);
+ goto free_expr;
+ }
+ expr->name = expr_name;
}
return expr;
--
2.43.0
