Re: [PATCH 14/15] perf/core: Add event_caps dependency flags

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 9/28/2026 4:05 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] perf/core: Userspace-triggerable WARN_ON_ONCE via retained event_caps in promoted group leader
> --
>
> commit ee7032954fb61d16c3b9fceb7931ce04cf4b8852
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>     
> perf/core: Add event_caps dependency flags
>     
> This commit adds two new event capability flags, PERF_EV_CAP_RELIED_ON and
> PERF_EV_CAP_RELIANT, to track dependencies between perf events in a group.
> When a relied-on event is detached, all reliant events are scheduled out
> and transitioned to an ERROR state.
>
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index de05df65ab3d0..55db0ad8b93a0 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -2349,13 +2349,12 @@ static void perf_promote_sibling_to_leader(struct perf_event *sibling,
>>  					   int group_caps)
>>  {
>>  	/*
>> -	 * Events that have PERF_EV_CAP_SIBLING require being part of
>> -	 * a group and cannot exist on their own, schedule them out
>> -	 * and move them into the ERROR state. Also see
>> -	 * _perf_event_enable(), it will not be able to recover this
>> -	 * ERROR state.
>> +	 * Events with PERF_EV_CAP_SIBLING or PERF_EV_CAP_RELIANT
>> +	 * must stay in a group or depend on another event in the
>> +	 * group; otherwise schedule them out and move them to ERROR.
>> +	 * _perf_event_enable() cannot recover from this state.
>>  	 */
>> -	if (sibling->event_caps & PERF_EV_CAP_SIBLING)
>> +	if (sibling->event_caps & (PERF_EV_CAP_SIBLING | PERF_EV_CAP_RELIANT))
>>  		__event_disable(sibling, ctx, PERF_EVENT_STATE_ERROR);
>>  
>>  	sibling->group_leader = sibling;
>>  	sibling->group_caps = group_caps;
> [Severity: High]
> Does this code need to clear PERF_EV_CAP_RELIED_ON and PERF_EV_CAP_RELIANT
> from sibling->event_caps when promoting the sibling to a singleton group
> leader?
>
> Since the flags are retained, a newly promoted leader can still carry
> PERF_EV_CAP_RELIANT.

Per my understanding, the event->event_caps should only be initialized at
the event creation and can't be modified from then. The event->event_caps
carries the specific capabilities of the event, it should not be changed or
cleared.

Besides, if PERF_EV_CAP_RELIANT is cleared for the metric events, the
metric events could be incorrectly enabled in _perf_event_enable().



>
>> @@ -2371,6 +2370,43 @@ static void perf_promote_sibling_to_leader(struct perf_event *sibling,
>>  	perf_event__header_size(sibling);
>>  }
>>  
>> +static void perf_group_disable_reliants(struct perf_event *event)
>> +{
>> +	struct perf_event *leader = event->group_leader;
>> +	struct perf_event *sibling, *tmp;
>> +	struct perf_event_context *ctx = event->ctx;
>> +	u32 mask = PERF_EV_CAP_RELIED_ON | PERF_EV_CAP_RELIANT;
>> +
>> +	if (!(event->event_caps & PERF_EV_CAP_RELIED_ON))
>> +		return;
>> +
>> +	/* Group leader should not carry PERF_EV_CAP_RELIED_ON. */
>> +	WARN_ON_ONCE(leader->event_caps & PERF_EV_CAP_RELIED_ON);
>> +	/* Group leader should not carry PERF_EV_CAP_RELIANT. */
>> +	WARN_ON_ONCE(leader->event_caps & PERF_EV_CAP_RELIANT);
> [Severity: High]
> Can userspace trigger this WARN_ON_ONCE because of the retained caps when
> siblings are promoted to leaders?
>
> If a user creates a perf event group with a non-slots leader, a slots
> sibling, and a metric sibling, the metric sibling can acquire
> PERF_EV_CAP_RELIANT.
>
> When the user closes the leader, the metric sibling is promoted to its
> own group leader, retaining PERF_EV_CAP_RELIANT and entering
> PERF_EVENT_STATE_ERROR.
>
> If the user then opens a new slots sibling and a new metric sibling under
> this promoted leader, and finally closes the new slots sibling, won't
> perf_group_disable_reliants() hit this WARN_ON_ONCE because the leader
> still holds the reliant flag? On systems with panic_on_warn enabled,
> this could cause a kernel panic.

It's a possible case, but won't it justify why the WARN_ON_ONCE() is
needed? Once such incorrect cases occur, users would be warned.

Thanks.


>