linux-kernel - Re: [PATCH 1/1] perf/core: fix dangling cgroup pointer in cpuctx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250604154639.GE8020@e132581.arm.com>
Date: Wed, 4 Jun 2025 16:46:39 +0100
From: Leo Yan <leo.yan@....com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Yeoreum Yun <yeoreum.yun@....com>, mingo@...hat.com, mingo@...nel.org,
	acme@...nel.org, namhyung@...nel.org, mark.rutland@....com,
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
	irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org, David Wang <00107082@....com>
Subject: Re: [PATCH 1/1] perf/core: fix dangling cgroup pointer in cpuctx

On Wed, Jun 04, 2025 at 04:16:40PM +0200, Peter Zijlstra wrote:

[...]

> It might be prudent to do something like so:

Thanks for the patch.

> +static void __event_disable(struct perf_event *event,
> +			    struct perf_event_context *ctx,
> +			    enum perf_event_state state)
> +{
> +	if (event == event->group_leader)
> +		group_sched_out(event, ctx);

I am a bit struggle for this code line. It disables all events in a
group, but only clear cgroup pointer for group leader but miss to clear
for sibling events.

If the cgroup pointer is only used for group leader, maybe we only
maintain (set and clear) the cgroup pointer for the leader?

Thanks,
Leo

> +	else
> +		event_sched_out(event, ctx);
> +
> +	perf_event_set_state(event, state);
> +	perf_cgroup_event_disable(event, ctx);
> +}
> +
>  /*
>   * Cross CPU call to disable a performance event
>   */
> @@ -2575,15 +2583,7 @@ static void __perf_event_disable(struct perf_event *event,
>  
>  	perf_pmu_disable(event->pmu_ctx->pmu);
>  	ctx_time_update_event(ctx, event);
> -
> -	if (event == event->group_leader)
> -		group_sched_out(event, ctx);
> -	else
> -		event_sched_out(event, ctx);
> -
> -	perf_event_set_state(event, PERF_EVENT_STATE_OFF);
> -	perf_cgroup_event_disable(event, ctx);
> -
> +	__event_disable(event, ctx, PERF_EVENT_STATE_OFF);
>  	perf_pmu_enable(event->pmu_ctx->pmu);
>  }
>