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: <19514ed5.5692.19734522326.Coremail.00107082@163.com>
Date: Tue, 3 Jun 2025 13:44:47 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Yeoreum Yun" <yeoreum.yun@....com>
Cc: 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, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] perf/core: fix dangling cgroup pointer in cpuctx

At 2025-06-03 12:46:03, "Yeoreum Yun" <yeoreum.yun@....com> wrote:
>Hi Davia,
>
>> I think this patch is no better than my patch in the original report
>> https://lore.kernel.org/all/20250601173603.3920-1-00107082@163.com/
>>
>> This patch is more aggressive,  it add more changes to original logic, same practice
>> as in the offending commit.  would raise more concerns about hidden side-effect.
>>
>> For example, this code  in list_del_event should raise concern about this patch
>> 2099          * We can have double detach due to exit/hot-unplug + close.
>>  2100          */
>>  2101         if (!(event->attach_state & PERF_ATTACH_CONTEXT))
>>  2102                 return;
>
>attach_state doesn't related for event->state change.
>if one event already cleared PERF_ATTACH_CONTEXT, that event is called
>via list_del_event()

Maybe this concern could be clarified, what about other subtle impacts.
The change should be thorough reviewed, if you want to push it further.

It takes me more than a month to figure out a procedure to reproduce the kernel panic bug,
It is  just very hard to capture a bug happens in rare situation.

And your patch has a global impact, it changes behavior unnecessarily.

>
>Also, your patch couldn't solve a problem describe in
>commit a3c3c6667("perf/core: Fix child_total_time_enabled accounting bug at task exit")
>for INCATIVE event's total_enable_time.

I do not think so.
Correct me if I am making silly  mistakes,
The patch, https://lore.kernel.org/lkml/20250603032651.3988-1-00107082@163.com/
calls perf_event_set_state() based on DETACH_EXIT flag, which cover the INACTIVE state, right?
If DETACH_EXIT is not used for this purpose? Then why should it exist at the first place?
I think I does not revert the purpose of commit a3c3c6667.....But I could be wrong
Would you show a call path where DETACH_EXIT is not set, but the changes in commit a3c3c6667 is still needed?

David

>
>Thanks.
>
>--
>Sincerely,
>Yeoreum Yun