Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

tracing/perf: Fix double put of trace event when init fails

If in perf_trace_event_init(), the perf_trace_event_open() fails, then it
will call perf_trace_event_unreg() which will not only unregister the perf
trace event, but will also call the put() function of the tp_event.

The problem here is that the trace_event_try_get_ref() is called by the
caller of perf_trace_event_init() and if perf_trace_event_init() returns a
failure, it will then call trace_event_put(). But since the
perf_trace_event_unreg() already called the trace_event_put() function, it
triggers a WARN_ON().

WARNING: CPU: 1 PID: 30309 at kernel/trace/trace_dynevent.c:46 trace_event_dyn_put_ref+0x15/0x20

If perf_trace_event_reg() does not call the trace_event_try_get_ref() then
the perf_trace_event_unreg() should not be calling trace_event_put(). This
breaks symmetry and causes bugs like these.

Pull out the trace_event_put() from perf_trace_event_unreg() and call it
in the locations that perf_trace_event_unreg() is called. This not only
fixes this bug, but also brings back the proper symmetry of the reg/unreg
vs get/put logic.

Link: https://lore.kernel.org/all/cover.1660347763.git.kjlx@templeofstupid.com/
Link: https://lkml.kernel.org/r/20220816192817.43d5e17f@gandalf.local.home

Cc: stable@vger.kernel.org
Fixes: 1d18538e6a092 ("tracing: Have dynamic events have a ref counter")
Reported-by: Krister Johansen <kjlx@templeofstupid.com>
Reviewed-by: Krister Johansen <kjlx@templeofstupid.com>
Tested-by: Krister Johansen <kjlx@templeofstupid.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

+4 -3
+4 -3
kernel/trace/trace_event_perf.c
··· 157 157 int i; 158 158 159 159 if (--tp_event->perf_refcount > 0) 160 - goto out; 160 + return; 161 161 162 162 tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL); 163 163 ··· 176 176 perf_trace_buf[i] = NULL; 177 177 } 178 178 } 179 - out: 180 - trace_event_put_ref(tp_event); 181 179 } 182 180 183 181 static int perf_trace_event_open(struct perf_event *p_event) ··· 239 241 mutex_lock(&event_mutex); 240 242 perf_trace_event_close(p_event); 241 243 perf_trace_event_unreg(p_event); 244 + trace_event_put_ref(p_event->tp_event); 242 245 mutex_unlock(&event_mutex); 243 246 } 244 247 ··· 291 292 mutex_lock(&event_mutex); 292 293 perf_trace_event_close(p_event); 293 294 perf_trace_event_unreg(p_event); 295 + trace_event_put_ref(p_event->tp_event); 294 296 mutex_unlock(&event_mutex); 295 297 296 298 destroy_local_trace_kprobe(p_event->tp_event); ··· 347 347 mutex_lock(&event_mutex); 348 348 perf_trace_event_close(p_event); 349 349 perf_trace_event_unreg(p_event); 350 + trace_event_put_ref(p_event->tp_event); 350 351 mutex_unlock(&event_mutex); 351 352 destroy_local_trace_uprobe(p_event->tp_event); 352 353 }