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

[Aditya Chillara]

On 6/29/2026 12:36 PM, Mi, Dapeng wrote:
> 
> On 6/29/2026 12:00 PM, Aditya Chillara wrote:
>> On 6/29/2026 8:28 AM, Mi, Dapeng wrote:
>>> On 6/26/2026 5:54 PM, Aditya Chillara wrote:
>>>> perf_group_detach() handles leader and sibling detach differently. When the
>>>> group leader is detached, all siblings are promoted to singleton events and
>>>> their group_leader pointer is reset to themselves. When a sibling is
>>>> detached, it is removed from the leader's sibling_list, but its
>>>> group_leader pointer is left pointing at the old leader.
>>>>
>>>> That is harmless when the sibling is being closed and freed immediately, as
>>>> in the DETACH_DEAD path. It is not safe when the sibling is detached but
>>>> kept alive, such as during CPU hotplug with DETACH_GROUP. In that case the
>>>> sibling is removed from the context, while its file descriptor can still
>>>> keep it alive.
>>>>
>>>> A typical failing sequence is:
>>>>
>>>>   - A group contains leader L and sibling S.
>>>>   - CPU hot-unplug detaches S with DETACH_GROUP, removing it from
>>>>     L->sibling_list but leaving S->group_leader == L.
>>>>   - L is later closed and freed.
>>>>   - A PERF_IOC_FLAG_GROUP ioctl on S follows S->group_leader and
>>>>     dereferences the freed leader.
>>>>
>>>> This was reproduced by running the perf event fuzzer, CPU hotplug, and a
>>>> stress workload concurrently:
>>>>
>>>> Unable to handle kernel paging request at virtual address 006b6b6b6b6b6cdb
>>>> CPU: 2 PID: 12489 Comm: perf_fuzzer 6.18.7 PREEMPT
>>>> pc : perf_ioctl+0x34c/0xc68
>>>> x20: ffffff89a3fa2c70 x8 : 6b6b6b6b6b6b6b6b
>>>> Code: 943c4a0e 340047a0 f9404a94 f9411e88 (f940b908)
>>>> Call trace:
>>>> perf_ioctl+0x34c/0xc68 (P)
>>>> __arm64_sys_ioctl+0xa0/0xf4
>>>> invoke_syscall+0x58/0xe4
>>>> el0_svc_common+0xa8/0xdc
>>>> do_el0_svc+0x1c/0x28
>>>> el0_svc+0x40/0xc0
>>>> el0t_64_sync_handler+0x68/0xdc
>>>> el0t_64_sync+0x1c4/0x1c8
>>>>
>>>> The fault happened in perf_ioctl(), where perf_event_for_each() follows
>>>> the stale group_leader pointer and perf_event_for_each_child() then
>>>> dereferences the freed leader's context.
>>>>
>>>> Fix the use-after-free by promoting the detached sibling to a singleton.
>>>>
>>>> Fixes: 8a49542c0554 ("perf_events: Fix races in group composition")
>>>> Assisted-by: PatchWise:gpt-5.5
>>>> Signed-off-by: Aditya Chillara <aditya.chillara@xxxxxxxxxxxxxxxx>
>>>> ---
>>>>  kernel/events/core.c | 20 ++++++++++++++++++++
>>>>  1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>>> index 954c36e28101..dd9892040ab2 100644
>>>> --- a/kernel/events/core.c
>>>> +++ b/kernel/events/core.c
>>>> @@ -2605,6 +2605,26 @@ __perf_remove_from_context(struct perf_event *event,
>>>>  		perf_child_detach(event);
>>>>  	list_del_event(event, ctx);
>>>>  
>>>> +	if ((flags & DETACH_GROUP) && event->group_leader != event) {
>>>> +		/*
>>>> +		 * list_del_event() needed the old group_leader to tell a real
>>>> +		 * leader from a sibling. That's done now, so make the detached
>>>> +		 * sibling self-contained.
>>>> +		 */
>>>> +		event->group_leader = event;
>>>> +		event->group_caps = event->event_caps;
>>>> +
>>>> +		/*
>>>> +		 * PERF_EV_CAP_SIBLING event requires being part of a group, so move
>>>> +		 * the event to ERROR state if it is still alive.
>>>> +		 */
>>>> +		if ((event->event_caps & PERF_EV_CAP_SIBLING) &&
>>>> +		    event->state > PERF_EVENT_STATE_ERROR)
>>>> +			perf_event_set_state(event, PERF_EVENT_STATE_ERROR);
>>>> +
>>>> +		perf_event__header_size(event);
>>>> +	}
>>>> +
>>> Why not move this part of fixing code into perf_group_detach()? It seems a
>>> better place to fix the issue. Thanks.
>> Because list_del_event() just above my change does:
>>
>> 	if (event->group_leader == event)
>> 		del_event_from_groups(event, ctx);
>>
>> so resetting the group leader in perf_group_detach() would attempt removing sibling
>> event->group_node from a group rb-tree it was never added to (only leader gets added
>> in list_add_event()).
> 
> Yeah, but I don't see why we can't do same thing for the sibling event
> detaching in perf_group_detach(). Just like the group leader detaching,
> each sibling event would be re-added into ctx groups by calling
> add_event_to_groups(). Suppose we can do same thing for the sibling event
> detaching, call add_event_to_groups() to add the standalone event into ctx
> groups, right?

Yup, that's a cleaner fix, sent v2!

Thank you,
Aditya

> 
> 
>         if (sibling->attach_state & PERF_ATTACH_CONTEXT) {
>             add_event_to_groups(sibling, event->ctx);
> 
>             if (sibling->state == PERF_EVENT_STATE_ACTIVE)
>                 list_add_tail(&sibling->active_list, get_event_list(sibling));
>         }
> 
> 
>>>
>>>>  	if (!pmu_ctx->nr_events) {
>>>>  		pmu_ctx->rotate_necessary = 0;
>>>>  
>>>>
>>>> ---
>>>> base-commit: ab9de95c9cf952332ab79453b4b5d1bfca8e514f
>>>> change-id: 20260626-fix-group-leader-uaf-c46960e525e0
>>>>
>>>> Best regards,
>>>> --  
>>>> Aditya Chillara <aditya.chillara@xxxxxxxxxxxxxxxx>
>>>>
>>>>
>>