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

[sashiko-bot@kernel.org]

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.

> @@ -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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928074309.898043-1-dapeng1.mi@linux.intel.com?part=14