linux-kernel - [PATCH 1/1] tracing: fix a WARN from trace_event_dyn_put

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]

Message-ID: <4e43a4eece5f382d1636397fb3c0208f2afe81fc.1660347763.git.kjlx@templeofstupid.com>
Date:   Fri, 12 Aug 2022 17:02:20 -0700
From:   Krister Johansen <kjlx@...pleofstupid.com>
To:     Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        David Reaver <me@...idreaver.com>, linux-kernel@...r.kernel.org
Subject: [PATCH 1/1] tracing: fix a WARN from trace_event_dyn_put_ref

The code in perf_trace_init takes a reference on a trace_event_call that is
looked up as part of the function call.  If perf_trace_event_int fails,
however, perf_trace_event_unreg can decrement that refcount from underneath
perf_trace_init.  This means that in some failure cases, perf_trace_init
can trigger the WARN in trace_dynevent.c which attempts to guard against
zero reference counts going negative.

The author can reproduce this problem readily by running perf record in a
loop against a series of uprobes with no other users.  Killing the record
process before it can finish its setup is enough to trigger this warn
within a few seconds.

This patch leaves the behavior in perf_trace_event_unreg unchanged, but
moves most of the code in that function to perf_trace_event_cleanup.  The
unreg function retains the ability to drop the refcount on the tp_event,
but cleanup does not.  This modification is based upon the observation that
all of the other callers of perf_trace_event_init don't bother with
manipulating a reference count on the tp_events that they create.  For
those callers, the trace_event_put_ref was already a no-op.

Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>
Reviewed-by: David Reaver <me@...idreaver.com>
Fixes: 1d18538e6a092 "tracing: Have dynamic events have a ref counter"
CC: stable@...r.kernel.org # 5.15, 5.18, 5.19
---
 kernel/trace/trace_event_perf.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a114549720d6..7762bfd268cd 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -151,13 +151,13 @@ static int perf_trace_event_reg(struct trace_event_call *tp_event,
 	return ret;
 }

-static void perf_trace_event_unreg(struct perf_event *p_event)
+static void perf_trace_event_cleanup(struct perf_event *p_event)
 {
 	struct trace_event_call *tp_event = p_event->tp_event;
 	int i;

 	if (--tp_event->perf_refcount > 0)
-		goto out;
+		return;

 	tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);

@@ -176,7 +176,13 @@ static void perf_trace_event_unreg(struct perf_event *p_event)
 			perf_trace_buf[i] = NULL;
 		}
 	}
-out:
+}
+
+static void perf_trace_event_unreg(struct perf_event *p_event)
+{
+	struct trace_event_call *tp_event = p_event->tp_event;
+
+	perf_trace_event_cleanup(p_event);
 	trace_event_put_ref(tp_event);
 }

@@ -207,7 +213,7 @@ static int perf_trace_event_init(struct trace_event_call *tp_event,

 	ret = perf_trace_event_open(p_event);
 	if (ret) {
-		perf_trace_event_unreg(p_event);
+		perf_trace_event_cleanup(p_event);
 		return ret;
 	}

-- 
2.25.1