linux-kernel - [PATCH] tracing/histogram: Fix UAF in destroy_hist

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211117021223.2137117-1-kaleshsingh@google.com>
Date:   Tue, 16 Nov 2021 18:12:22 -0800
From:   Kalesh Singh <kaleshsingh@...gle.com>
To:     unlisted-recipients:; (no To-header on input)
Cc:     kernel-team@...roid.com, rostedt@...dmis.org, mhiramat@...nel.org,
        zanussi@...nel.org, Kalesh Singh <kaleshsingh@...gle.com>,
        kernel test robot <oliver.sang@...el.com>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org
Subject: [PATCH] tracing/histogram: Fix UAF in destroy_hist_field()

Calling destroy_hist_field() on an expression will recursively free
any operands associated with the expression. If during expression
parsing the operands of the expression are already set when an error
is encountered, there is no need to explicity free the operands. Doing
so will result in destroy_hist_field() being called twice for the
operands and lead to a use-after-free (UAF) error.

Fix this by only calling destroy_hist_field() for the expression if the
operands are already set.

Signed-off-by: Kalesh Singh <kaleshsingh@...gle.com>
Fixes: 8b5d46fd7a38 ("tracing/histogram: Optimize division by constants")
Reported-by: kernel test robot <oliver.sang@...el.com>
---
 kernel/trace/trace_events_hist.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 5ea2c9ec54a6..e3856eaf2ac3 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2669,7 +2669,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		if (!divisor) {
 			hist_err(file->tr, HIST_ERR_DIVISION_BY_ZERO, errpos(str));
 			ret = -EDOM;
-			goto free;
+			goto free_expr;
 		}
 
 		/*
@@ -2709,7 +2709,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 		expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
 		if (!expr->type) {
 			ret = -ENOMEM;
-			goto free;
+			goto free_expr;
 		}
 
 		expr->name = expr_str(expr, 0);
@@ -2719,6 +2719,7 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
 free:
 	destroy_hist_field(operand1, 0);
 	destroy_hist_field(operand2, 0);
+free_expr:
 	destroy_hist_field(expr, 0);
 
 	return ERR_PTR(ret);

base-commit: 8ab774587903771821b59471cc723bba6d893942
-- 
2.34.0.rc1.387.gb447b232ab-goog