Re: [PATCH v2] perf/core: Fix group leader use-after-free after sibling detach

[Peter Zijlstra <peterz@infradead.org>]

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.