Double perf_trace_event_unreg is allowed causing perf_refcount to go
negative. total_ref_count also goes negative because the return value
of tracepoint_probe_unregister is ignored.
Once total_ref_count is negative, the next call to perf_trace_event_reg
will register perf_probe but will not allocate perf_trace_buf and sets
it to NULL instead.
The subsequent trace_##call() will mem abort in perf_trace_buf_alloc
because memset will be called on the NULL perf_trace_buf.
Gracefully handle the error in perf_trace_event_unreg to prevent
double unregister.
Signed-off-by: Aditya Chillara <quic_achillar@quicinc.com>
---
kernel/trace/trace_event_perf.c | 8 ++++++--
kernel/trace/trace_events.c | 3 +--
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 61e3a2620fa3c9417ac23cf5a18aeb86e7393dcc..247db88accd88eb0acf3692ea593d576519ce8b1 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -154,12 +154,16 @@ static int perf_trace_event_reg(struct trace_event_call *tp_event,
static void perf_trace_event_unreg(struct perf_event *p_event)
{
struct trace_event_call *tp_event = p_event->tp_event;
- int i;
+ int i, ret;
if (--tp_event->perf_refcount > 0)
return;
- tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
+ ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
+ if (ret) {
+ ++tp_event->perf_refcount;
+ return;
+ }
/*
* Ensure our callback won't be called anymore. The buffers
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0356cae0cf74e79075f607bc841df05568688baa..50e0e08b29aa6617a04b191419ad1e587adf69fe 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -530,10 +530,9 @@ int trace_event_reg(struct trace_event_call *call,
call->class->perf_probe,
call);
case TRACE_REG_PERF_UNREGISTER:
- tracepoint_probe_unregister(call->tp,
+ return tracepoint_probe_unregister(call->tp,
call->class->perf_probe,
call);
- return 0;
case TRACE_REG_PERF_OPEN:
case TRACE_REG_PERF_CLOSE:
case TRACE_REG_PERF_ADD:
--
2.34.1On Wed, 9 Jul 2025 11:11:09 +0530
Aditya Chillara <quic_achillar@quicinc.com> wrote:
> Double perf_trace_event_unreg is allowed causing perf_refcount to go
> negative. total_ref_count also goes negative because the return value
> of tracepoint_probe_unregister is ignored.
>
> Once total_ref_count is negative, the next call to perf_trace_event_reg
> will register perf_probe but will not allocate perf_trace_buf and sets
> it to NULL instead.
>
> The subsequent trace_##call() will mem abort in perf_trace_buf_alloc
> because memset will be called on the NULL perf_trace_buf.
>
> Gracefully handle the error in perf_trace_event_unreg to prevent
> double unregister.
>
> Signed-off-by: Aditya Chillara <quic_achillar@quicinc.com>
> ---
> kernel/trace/trace_event_perf.c | 8 ++++++--
> kernel/trace/trace_events.c | 3 +--
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
> index 61e3a2620fa3c9417ac23cf5a18aeb86e7393dcc..247db88accd88eb0acf3692ea593d576519ce8b1 100644
> --- a/kernel/trace/trace_event_perf.c
> +++ b/kernel/trace/trace_event_perf.c
> @@ -154,12 +154,16 @@ static int perf_trace_event_reg(struct trace_event_call *tp_event,
> static void perf_trace_event_unreg(struct perf_event *p_event)
> {
> struct trace_event_call *tp_event = p_event->tp_event;
> - int i;
> + int i, ret;
>
> if (--tp_event->perf_refcount > 0)
> return;
>
> - tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
> + ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
The only time unreg() fails is when it doesn't find a tracepoint to
unregister.
There should be no reason to check the return value of unregister if
you have your accounting correct. Thus I think you are fixing a symptom
of a bug elsewhere.
-- Steve
> + if (ret) {
> + ++tp_event->perf_refcount;
> + return;
> + }
>
On 7/9/2025 7:53 PM, Steven Rostedt wrote:
> On Wed, 9 Jul 2025 11:11:09 +0530
> Aditya Chillara <quic_achillar@quicinc.com> wrote:
>
>> Double perf_trace_event_unreg is allowed causing perf_refcount to go
>> negative. total_ref_count also goes negative because the return value
>> of tracepoint_probe_unregister is ignored.
>>
>> Once total_ref_count is negative, the next call to perf_trace_event_reg
>> will register perf_probe but will not allocate perf_trace_buf and sets
>> it to NULL instead.
>>
>> The subsequent trace_##call() will mem abort in perf_trace_buf_alloc
>> because memset will be called on the NULL perf_trace_buf.
>>
>> Gracefully handle the error in perf_trace_event_unreg to prevent
>> double unregister.
>>
>> Signed-off-by: Aditya Chillara <quic_achillar@quicinc.com>
>> ---
>> kernel/trace/trace_event_perf.c | 8 ++++++--
>> kernel/trace/trace_events.c | 3 +--
>> 2 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
>> index 61e3a2620fa3c9417ac23cf5a18aeb86e7393dcc..247db88accd88eb0acf3692ea593d576519ce8b1 100644
>> --- a/kernel/trace/trace_event_perf.c
>> +++ b/kernel/trace/trace_event_perf.c
>> @@ -154,12 +154,16 @@ static int perf_trace_event_reg(struct trace_event_call *tp_event,
>> static void perf_trace_event_unreg(struct perf_event *p_event)
>> {
>> struct trace_event_call *tp_event = p_event->tp_event;
>> - int i;
>> + int i, ret;
>>
>> if (--tp_event->perf_refcount > 0)
>> return;
>>
>> - tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
>> + ret = tp_event->class->reg(tp_event, TRACE_REG_PERF_UNREGISTER, NULL);
>
> The only time unreg() fails is when it doesn't find a tracepoint to
> unregister.
>
> There should be no reason to check the return value of unregister if
> you have your accounting correct. Thus I think you are fixing a symptom
> of a bug elsewhere.
The exact problem was introduced by:
https://github.com/torvalds/linux/commit/7ef5aa081f989ecfecc1df02068a80aebbd3ec31
(perf/core: Simplify the perf_event_alloc() error path)
where __free_event was calling event->destroy() even though it would
have been called by perf_try_init_event in case it failed.
This was fixed by:
https://github.com/torvalds/linux/commit/da02f54e81db2f7bf6af9d1d0cfc5b41ec6d0dcb
(perf/core: Clean up perf_try_init_event())
This patch prevents from crashing even if that happens, and there
will be a warning anyway to notice the double unregister.
>
> -- Steve
>
>
>> + if (ret) {
>> + ++tp_event->perf_refcount;
>> + return;
>> + }
>>
Best Regards,
Aditya
On Wed, 9 Jul 2025 22:20:00 +0530 Aditya Chillara <quic_achillar@quicinc.com> wrote: > The exact problem was introduced by: > https://github.com/torvalds/linux/commit/7ef5aa081f989ecfecc1df02068a80aebbd3ec31 > (perf/core: Simplify the perf_event_alloc() error path) > where __free_event was calling event->destroy() even though it would > have been called by perf_try_init_event in case it failed. Then I rather have it trigger a WARN_ON() and disable that event permanently until reboot. It's a bug, no need to continue using the event when it's in an a bad state. -- Steve
[ Added Peter for 7ef5aa081f98 and kernel/events/core.c ] On 7/9/2025 10:48 PM, Steven Rostedt wrote: > On Wed, 9 Jul 2025 22:20:00 +0530 > Aditya Chillara <quic_achillar@quicinc.com> wrote: > >> The exact problem was introduced by: >> https://github.com/torvalds/linux/commit/7ef5aa081f989ecfecc1df02068a80aebbd3ec31 >> (perf/core: Simplify the perf_event_alloc() error path) >> where __free_event was calling event->destroy() even though it would >> have been called by perf_try_init_event in case it failed. > > Then I rather have it trigger a WARN_ON() and disable that event > permanently until reboot. It's a bug, no need to continue using the > event when it's in an a bad state. perf_trace_event_unreg is called only in event->destroy(), and this is called in event free path; either after the event is removed from its context, or before the event is installed in a context. I believe there is no need to explicitly disable the perf event here because it must have been disabled already. Please let me know if I'm missing anything. Best Regards, Aditya
On 7/9/2025 10:48 PM, Steven Rostedt wrote: >> The exact problem was introduced by: >> https://github.com/torvalds/linux/commit/7ef5aa081f989ecfecc1df02068a80aebbd3ec31 >> (perf/core: Simplify the perf_event_alloc() error path) >> where __free_event was calling event->destroy() even though it would >> have been called by perf_try_init_event in case it failed. > Then I rather have it trigger a WARN_ON() and disable that event > permanently until reboot. It's a bug, no need to continue using the > event when it's in an a bad state. Acknowledged, will update in v3. Best Regards, Aditya
© 2016 - 2026 Red Hat, Inc.