Re: [PATCH v2] perf/core: Fix group leader use-after-free after sibling detach
From: Peter Zijlstra
Date: Fri Aug 07 2026 - 05:02:36 EST
On Fri, Aug 07, 2026 at 12:24:20PM +0530, Aditya Chillara wrote:
> > The below code used __event_disable(); and this change is not
> > mentioned in the Changelog. Why was this changed?
>
> I should've mentioned this in the Changelog, I'll update it.
>
> In case of __perf_remove_from_context(event, ...) where the event is part of a group
> but is not the leader; by the time we get to perf_group_detach():
>
> 1. perf_cgroup_event_disable(event, ctx) would have been called
> 2. The sibling's state may have been set to EXIT/REVOKED/DEAD if not OFF
>
> So we cannot use __event_disable() because it would unconditionally call
> 1. perf_cgroup_event_disable() that would decrement nr_cgroups twice
> 2. perf_event_set_state(event, PERF_EVENT_STATE_ERROR) that could bring back
> the state from EXIT/REVOKED/DEAD/OFF to ERROR.
>
> In previous code, in case of __perf_remove_from_context(event, ...) where the event
> is a leader; __event_disable() was called for siblings unconditionally but I do not
> see a reason why the added guards don't hold.
>
> Should we move the guards to __event_disable() instead? That would simplify the code:
>
> static void __event_disable(struct perf_event *event,
> struct perf_event_context *ctx,
> enum perf_event_state state)
> {
> event_sched_out(event, ctx);
> if (event->state > PERF_EVENT_STATE_OFF)
> perf_cgroup_event_disable(event, ctx);
> perf_event_set_state(event, min(event->state, state));
> }
Yes, that looks good.