linux-kernel - Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of event_trigger

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 24 Jul 2018 19:13:31 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Tom Zanussi <tom.zanussi@...ux.intel.com>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        Ingo Molnar <mingo@...hat.com>, Shuah Khan <shuah@...nel.org>,
        Hiraku Toyooka <hiraku.toyooka@...ertrust.co.jp>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 1/3] [BUGFIX] tracing: Fix double free of
 event_trigger_data

On Tue, 24 Jul 2018 17:30:08 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> I don't see where ->reg() would return anything but 1 on success. Maybe
> I'm missing something. I'll look some more, but I'm thinking of changing
> ->reg() to return zero on all success, and negative on all errors and  
> just check those results.

Yeah, I'm going to make these changes. That is, all struct
event_command .reg functions will return negative on error, and
zero or positive on everything else. All the checks are going to be
only for the negative value.

But for the bug fix itself, I believe this should do it. Masami, what
do you think?

-- Steve

diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index d18249683682..8771a27ca60f 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -679,6 +679,8 @@ event_trigger_callback(struct event_command *cmd_ops,
 		goto out_free;
 
  out_reg:
+	/* Up the trigger_data count to make sure reg doesn't free it on failure */
+	event_trigger_init(trigger_ops, trigger_data);
 	ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
 	/*
 	 * The above returns on success the # of functions enabled,
@@ -687,10 +689,11 @@ event_trigger_callback(struct event_command *cmd_ops,
 	 */
 	if (!ret) {
 		ret = -ENOENT;
-		goto out_free;
-	} else if (ret < 0)
-		goto out_free;
-	ret = 0;
+	} else if (ret > 0)
+		ret = 0;
+
+	/* Down the counter of trigger_data or free it if not used anymore */
+	event_trigger_free(trigger_ops, trigger_data);
  out:
 	return ret;