On Wed, 14 Oct 2026, Steven Rostedt wrote:

> On Wed, 14 Oct 2026 10:30:12 +0000
> Lee Jones <[email protected]> wrote:
> 
> > In event_hist_trigger_parse() and hist_unreg_all(), the histogram
> > registration and unregistration paths look up find_synth_event() using
> > trace_event_name(file->event_call) and adjust se->ref if a match is
> > found.
> > 
> > Because find_synth_event() only compares the event basename, registering
> > or removing a histogram on a non-synthetic trace event whose basename
> > matches a synthetic event erroneously modifies the synthetic event's
> > reference count.  If a histogram is registered on the non-synthetic
> > event before the synthetic event is created and removed afterwards,
> > se->ref is decremented without a matching increment, allowing the
> > synthetic event to be freed while an active histogram action still holds
> > a pointer to it.
> > 
> > Verify that &se->call matches file->event_call before adjusting se->ref.
> > 
> > Fixes: 4b147936fa50 ("tracing: Add support for 'synthetic' events")
> > Cc: [email protected]
> > Signed-off-by: Lee Jones <[email protected]>
> 
> Hi,
> 
> Thanks for the patch...
> 
> > ---
> >  kernel/trace/trace_events_hist.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_events_hist.c 
> > b/kernel/trace/trace_events_hist.c
> > index 00a1d8522036..71b240b609ea 100644
> > --- a/kernel/trace/trace_events_hist.c
> > +++ b/kernel/trace/trace_events_hist.c
> > @@ -6807,7 +6807,7 @@ static void hist_unreg_all(struct trace_event_file 
> > *file)
> >  
> >                     se_name = trace_event_name(file->event_call);
> >                     se = find_synth_event(se_name);
> > -                   if (se)
> > +                   if (se && &se->call == file->event_call)
> >                             se->ref--;
> >  
> >                     update_cond_flag(file);
> > @@ -6926,7 +6926,7 @@ static int event_hist_trigger_parse(struct 
> > event_command *cmd_ops,
> >             event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
> >             se_name = trace_event_name(file->event_call);
> >             se = find_synth_event(se_name);
> > -           if (se)
> > +           if (se && &se->call == file->event_call)
> >                     se->ref--;
> >             ret = 0;
> >             goto out_free;
> > @@ -6962,7 +6962,7 @@ static int event_hist_trigger_parse(struct 
> > event_command *cmd_ops,
> >  
> >     se_name = trace_event_name(file->event_call);
> >     se = find_synth_event(se_name);
> > -   if (se)
> > +   if (se && &se->call == file->event_call)
> >             se->ref++;
> >   out:
> >     if (ret == 0 && glob[0])
> 
> Instead of open coding each of these, can we add a new helper function:
> 
> struct synth_event *find_synth_event_call(struct trace_event_call *call)
> {
>       struct synth_event *se;
>       const char *name;
> 
>       name = trace_event_name(call);
>       se = find_synth_event(name);
>       return se && se->call == call ? se : NULL;
> }
> 
> The above can be defined in trace_events_synth.c and exported like
> find_synth_event() is.
> 
> Then we can remove the se_name from each of these functions.

Thanks for the pointer.  I'll look into it.

-- 
Lee Jones

Reply via email to