ACK/Cmnt: [PATCH 1/1][linux-5.15] tracing/perf: Fix double put of trace event when init fails

[Krister Johansen]

On Mon, Aug 29, 2022 at 09:35:05AM +0200, Stefan Bader wrote:
> On 28.08.22 04:02, Krister Johansen wrote:
> 
> BugLink: https://bugs.launchpad.net/bugs/1987232
> 
> > commit 7249921d94ff64f67b733eca0b68853a62032b3d upstream.
> > 
> > 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 at vger.kernel.org
> > Fixes: 1d18538e6a092 ("tracing: Have dynamic events have a ref counter")
> > Reported-by: Krister Johansen <kjlx at templeofstupid.com>
> > Reviewed-by: Krister Johansen <kjlx at templeofstupid.com>
> > Tested-by: Krister Johansen <kjlx at templeofstupid.com>
> > Acked-by: Jiri Olsa <jolsa at kernel.org>
> > Signed-off-by: Steven Rostedt (Google) <rostedt at goodmis.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> 
> (cherry picked from commit 0d7970e8702b7d15293a844cb59ddd352c298f26 linux-5.15.y)
> > Signed-off-by: Krister Johansen <kjlx at templeofstupid.com>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> > ---
> 
> Hi Krister,
> 
> since a new cycle just stated and we are still about 10 releases of upstream
> stable behind I would guess this would probably still be a rather long time
> until it hits the normal way. So picking it up before sounds reasonable.
> Things need a little addition for process but that is not a big problem.


Hi Stefan,
I appreciate your willingness to pick this up ahead of 5.15.63 making it
into the SRU cycle.

Thanks again!

-K