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: <aD77ih5zOrZyXVit@e129823.arm.com>
Date: Tue, 3 Jun 2025 14:41:30 +0100
From: Yeoreum Yun <yeoreum.yun@....com>
To: peterz@...radead.org, mingo@...hat.com, mingo@...nel.org,
	acme@...nel.org, namhyung@...nel.org, leo.yan@....com,
	mark.rutland@....com, alexander.shishkin@...ux.intel.com,
	jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com
Cc: 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

Hi David,

> >
> > > But to fix it,  isn't following change less aggressive?
> > >         event_sched_out(event, ctx);
> > > -       perf_event_set_state(event, min(event->state, state));
> > >         if (flags & DETACH_GROUP)
> > >                 perf_group_detach(event);
> > >         if (flags & DETACH_CHILD)
> > >                perf_child_detach(event);
> > >         list_del_event(event, ctx);
> > > +       perf_event_set_state(event, min(event->state, state));
> >
> > If perf_child_detach() is called first and perf_event_set_state() call,
> > since the parent is removed in perf_child_detatced,
> > It would be failed to account the total_enable_time which caculating
> > child_event's enable_time too.
>
> Thanks for clarifying this,
> So the whole point  in commit a3c3c6667 is to make  perf_event_set_state() happens before perf_child_detach(), right?
> I feel I got lost somewhere when I rush to this suggestion. But I still don't understand why my patchv1  breaks commit
> a3c3c6667, really confused.

I explained this in:
   https://lore.kernel.org/all/5d17f1d7.666d.197348b78d1.Coremail.00107082@163.com/

>> If there is specific child cpu event specified in cpu 0.
>>   1. cpu 0 -> active
>>   2. scheulded to cpu1 -> inactive
>>   3. close the cpu event from parent -> inactive close
>>
>> Can be failed to count total_enable_time.


Consider one event which attached to taskctx with specific cpu.
In case of your original patch is for only "DETACH_EXIT" case.
Here what I mean, the event is "closed".
In this case, based on your patch, it doesn't call the perf_event_set_state()
before list_del_event(), but perf_event_set_state() is called after list_del_event().

Thanks

--
Sincerely,
Yeoreum Yun