Tracing fixes:

- Fix double free in destroy_hist_field
 
  - Harden memset() of trace_iterator structure
 
  - Do not warn in trace printk check when test buffer fills up
 -----BEGIN PGP SIGNATURE-----
 
 iIoEABYIADIWIQRRSw7ePDh/lE+zeZMp5XQQmuv6qgUCYZgSTRQccm9zdGVkdEBn
 b29kbWlzLm9yZwAKCRAp5XQQmuv6qqsJAQDg6Oe0XMclYPLMyRlEJEMEV2bFh8ZQ
 G1jqvMLcMnuFZAEA2onhzHzjR1amXuSw9YwNHcDB7eHiaIg9pgdOFFDUpwI=
 =KTcf
 -----END PGP SIGNATURE-----

Merge tag 'trace-v5.16-6' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace

Pull tracing fixes from Steven Rostedt:

 - Fix double free in destroy_hist_field

 - Harden memset() of trace_iterator structure

 - Do not warn in trace printk check when test buffer fills up

* tag 'trace-v5.16-6' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
  tracing: Don't use out-of-sync va_list in event printing
  tracing: Use memset_startat() to zero struct trace_iterator
  tracing/histogram: Fix UAF in destroy_hist_field()
This commit is contained in:
Linus Torvalds 2021-11-19 13:50:48 -08:00
commit e4365e369f
2 changed files with 36 additions and 23 deletions

View File

@ -3812,6 +3812,18 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt,
iter->fmt[i] = '\0'; iter->fmt[i] = '\0';
trace_seq_vprintf(&iter->seq, iter->fmt, ap); trace_seq_vprintf(&iter->seq, iter->fmt, ap);
/*
* If iter->seq is full, the above call no longer guarantees
* that ap is in sync with fmt processing, and further calls
* to va_arg() can return wrong positional arguments.
*
* Ensure that ap is no longer used in this case.
*/
if (iter->seq.full) {
p = "";
break;
}
if (star) if (star)
len = va_arg(ap, int); len = va_arg(ap, int);
@ -6706,9 +6718,7 @@ waitagain:
cnt = PAGE_SIZE - 1; cnt = PAGE_SIZE - 1;
/* reset all but tr, trace, and overruns */ /* reset all but tr, trace, and overruns */
memset(&iter->seq, 0, memset_startat(iter, 0, seq);
sizeof(struct trace_iterator) -
offsetof(struct trace_iterator, seq));
cpumask_clear(iter->started); cpumask_clear(iter->started);
trace_seq_init(&iter->seq); trace_seq_init(&iter->seq);
iter->pos = -1; iter->pos = -1;

View File

@ -2576,28 +2576,27 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
/* Split the expression string at the root operator */ /* Split the expression string at the root operator */
if (!sep) if (!sep)
goto free; return ERR_PTR(-EINVAL);
*sep = '\0'; *sep = '\0';
operand1_str = str; operand1_str = str;
str = sep+1; str = sep+1;
/* Binary operator requires both operands */ /* Binary operator requires both operands */
if (*operand1_str == '\0' || *str == '\0') if (*operand1_str == '\0' || *str == '\0')
goto free; return ERR_PTR(-EINVAL);
operand_flags = 0; operand_flags = 0;
/* LHS of string is an expression e.g. a+b in a+b+c */ /* LHS of string is an expression e.g. a+b in a+b+c */
operand1 = parse_expr(hist_data, file, operand1_str, operand_flags, NULL, n_subexprs); operand1 = parse_expr(hist_data, file, operand1_str, operand_flags, NULL, n_subexprs);
if (IS_ERR(operand1)) { if (IS_ERR(operand1))
ret = PTR_ERR(operand1); return ERR_CAST(operand1);
operand1 = NULL;
goto free;
}
if (operand1->flags & HIST_FIELD_FL_STRING) { if (operand1->flags & HIST_FIELD_FL_STRING) {
hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(operand1_str)); hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(operand1_str));
ret = -EINVAL; ret = -EINVAL;
goto free; goto free_op1;
} }
/* RHS of string is another expression e.g. c in a+b+c */ /* RHS of string is another expression e.g. c in a+b+c */
@ -2605,13 +2604,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
operand2 = parse_expr(hist_data, file, str, operand_flags, NULL, n_subexprs); operand2 = parse_expr(hist_data, file, str, operand_flags, NULL, n_subexprs);
if (IS_ERR(operand2)) { if (IS_ERR(operand2)) {
ret = PTR_ERR(operand2); ret = PTR_ERR(operand2);
operand2 = NULL; goto free_op1;
goto free;
} }
if (operand2->flags & HIST_FIELD_FL_STRING) { if (operand2->flags & HIST_FIELD_FL_STRING) {
hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(str)); hist_err(file->tr, HIST_ERR_INVALID_STR_OPERAND, errpos(str));
ret = -EINVAL; ret = -EINVAL;
goto free; goto free_operands;
} }
switch (field_op) { switch (field_op) {
@ -2629,12 +2627,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
break; break;
default: default:
ret = -EINVAL; ret = -EINVAL;
goto free; goto free_operands;
} }
ret = check_expr_operands(file->tr, operand1, operand2, &var1, &var2); ret = check_expr_operands(file->tr, operand1, operand2, &var1, &var2);
if (ret) if (ret)
goto free; goto free_operands;
operand_flags = var1 ? var1->flags : operand1->flags; operand_flags = var1 ? var1->flags : operand1->flags;
operand2_flags = var2 ? var2->flags : operand2->flags; operand2_flags = var2 ? var2->flags : operand2->flags;
@ -2653,12 +2651,13 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
expr = create_hist_field(hist_data, NULL, flags, var_name); expr = create_hist_field(hist_data, NULL, flags, var_name);
if (!expr) { if (!expr) {
ret = -ENOMEM; ret = -ENOMEM;
goto free; goto free_operands;
} }
operand1->read_once = true; operand1->read_once = true;
operand2->read_once = true; operand2->read_once = true;
/* The operands are now owned and free'd by 'expr' */
expr->operands[0] = operand1; expr->operands[0] = operand1;
expr->operands[1] = operand2; expr->operands[1] = operand2;
@ -2669,7 +2668,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
if (!divisor) { if (!divisor) {
hist_err(file->tr, HIST_ERR_DIVISION_BY_ZERO, errpos(str)); hist_err(file->tr, HIST_ERR_DIVISION_BY_ZERO, errpos(str));
ret = -EDOM; ret = -EDOM;
goto free; goto free_expr;
} }
/* /*
@ -2709,18 +2708,22 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
expr->type = kstrdup_const(operand1->type, GFP_KERNEL); expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
if (!expr->type) { if (!expr->type) {
ret = -ENOMEM; ret = -ENOMEM;
goto free; goto free_expr;
} }
expr->name = expr_str(expr, 0); expr->name = expr_str(expr, 0);
} }
return expr; return expr;
free:
destroy_hist_field(operand1, 0);
destroy_hist_field(operand2, 0);
destroy_hist_field(expr, 0);
free_operands:
destroy_hist_field(operand2, 0);
free_op1:
destroy_hist_field(operand1, 0);
return ERR_PTR(ret);
free_expr:
destroy_hist_field(expr, 0);
return ERR_PTR(ret); return ERR_PTR(ret);
} }