| 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
| ||
|
Message-ID: <Zd/86VJB/EQZsYPD@xpf.sh.intel.com>
Date: Thu, 29 Feb 2024 11:41:29 +0800
From: Pengfei Xu <pengfei.xu@...el.com>
To: Ravi Bangoria <ravi.bangoria@....com>
CC: "peterz@...radead.org" <peterz@...radead.org>, "namhyung@...nel.org"
<namhyung@...nel.org>, "Eranian, Stephane" <eranian@...gle.com>,
"acme@...nel.org" <acme@...nel.org>, "mark.rutland@....com"
<mark.rutland@....com>, "jolsa@...nel.org" <jolsa@...nel.org>,
"irogers@...gle.com" <irogers@...gle.com>, "bp@...en8.de" <bp@...en8.de>,
"kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>, "Hunter, Adrian"
<adrian.hunter@...el.com>, "maddy@...ux.ibm.com" <maddy@...ux.ibm.com>,
"x86@...nel.org" <x86@...nel.org>, "linux-perf-users@...r.kernel.org"
<linux-perf-users@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "sandipan.das@....com"
<sandipan.das@....com>, "ananth.narayan@....com" <ananth.narayan@....com>,
"santosh.shukla@....com" <santosh.shukla@....com>, lkp <lkp@...el.com>
Subject: Re: [PATCH v4 1/4] perf/core: Rework forwarding of {task|cpu}-clock
events
On 2024-02-28 at 20:49:59 +0800, Ravi Bangoria wrote:
> >>> Currently, PERF_TYPE_SOFTWARE is treated specially since task-clock and
> >>> cpu-clock events are interfaced through it but internally gets forwarded
> >>> to their own pmus.
> >>>
> >>> Rework this by overwriting event->attr.type in perf_swevent_init() which
> >>> will cause perf_init_event() to retry with updated type and event will
> >>> automatically get forwarded to right pmu. With the change, SW pmu no
> >>> longer needs to be treated specially and can be included in 'pmu_idr'
> >>> list.
> >>>
> >>> Suggested-by: Peter Zijlstra <peterz@...radead.org>
> >>> Signed-off-by: Ravi Bangoria <ravi.bangoria@....com>
> >>> ---
> >>> include/linux/perf_event.h | 10 +++++
> >>> kernel/events/core.c | 77 ++++++++++++++++++++------------------
> >>> 2 files changed, 51 insertions(+), 36 deletions(-)
> >>
> >> Greeting!
> >> There is task hung in perf_tp_event_init in v6.8-rc4 in guest.
> >
> > Thanks for the bug report. I'm able to reproduce it. Will try to spend
> > more time to rootcause it.
>
> Although the bisect has lead to 0d6d062ca27e as culprit commit, a minor
> change (shown below) in the test program can create the same task hang
> issue even with 0d6d062ca27e reverted.
>
> - *(uint32_t*)0x200000c0 = 6; /* Use cpu-clock pmu type when 0d6d062ca27e is present */
> + *(uint32_t*)0x200000c0 = 1; /* Use software pmu type when 0d6d062ca27e is absent */
>
> So, 0d6d062ca27e is not the culprit commit.
Thank you for your analysis and additional verification!
commit 0d6d062ca27e only changes kernel/events/core.c judgement:
PERF_TYPE_SOFTWARE to perf_cpu_clock.type in function cpu_clock_event_init():
"
@@ -11086,7 +11092,7 @@ static void cpu_clock_event_read(struct perf_event *event)
static int cpu_clock_event_init(struct perf_event *event)
{
- if (event->attr.type != PERF_TYPE_SOFTWARE)
+ if (event->attr.type != perf_cpu_clock.type)
return -ENOENT;
"
After reverted the commit, and change test code 0x200000c0 from 6 to 1,
kernel code in kernel/events/core.c: func perf_swevent_init() still keeps
PERF_TYPE_SOFTWARE(1) judgement because above commit doesn't change judgement
in perf_swevent_init():
"
// PERF_TYPE_SOFTWARE = 1;
if (event->attr.type != PERF_TYPE_SOFTWARE)
return -ENOENT;
"
So it seems show that attr.type(0x200000c0) = 6 will return -2 and not solve
further action in perf_swevent_init() will not trigger task hang.
And if attr.type(0x200000c0) = 1 will pass the judgement and solve further
action in perf_swevent_init(), and then will trigger task hang.
Thanks for correction if there is something wrong.
>
> Additionally,
>
> o I've seen task hang or soft-lockups on a single cpu KVM guest while
> running your test as root and also as normal user with
> perf_event_paranoid=-1. But the same experiment on host, no lockups,
> only task hang. So I feel the bug report is false positive and there
> is no real issue (since the experiment requires special privilege).
Thanks for your info! Seems some problem will cause cpu soft-lockups.
If there is more clue, will update it.
>
> o 0d6d062ca27e has inadvertently started allowing cpu-clock and task-
> clock events creation via their own pmu->type in perf_event_open(),
> instead of earlier design where the only interface was through sw
> pmu. Is it harmful? Probably not. But worth to be documented:
Thanks a lot for description, and there is some other way to trigger
the perf_event type change.
Thanks!
>
> ----><----
>
> From c7ae1c57e2a23a05eb982524d37bc8542c9c9a34 Mon Sep 17 00:00:00 2001
> From: Ravi Bangoria <ravi.bangoria@....com>
> Date: Wed, 28 Feb 2024 17:29:04 +0530
> Subject: [PATCH] perf/core: Document {cpu|task}-clock event open behavior
>
> The standard interface to invoke task-clock and cpu-clock pmu is through
> software pmu (see perf_swevent_init()), since these pmus are not exposed
> to the user via sysfs and thus user doesn't know their pmu->type. However,
> current code allows user to open an event if user has passed correct type
> in the perf event attribute. This is not easily apparent from the code and
> thus worth to be documented.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@....com>
> ---
> kernel/events/core.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f0f0f71213a1..4072bccab3ba 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11178,6 +11178,13 @@ static void cpu_clock_event_read(struct perf_event *event)
>
> static int cpu_clock_event_init(struct perf_event *event)
> {
> + /*
> + * The standard interface to invoke task-clock pmu is through software
> + * pmu(see perf_swevent_init()), since task-clock pmu is not exposed to
> + * the user via sysfs and thus user doesn't know perf_task_clock.type.
> + * However, allow user to open an event if user has passed correct type
> + * in the attribute.
> + */
> if (event->attr.type != perf_cpu_clock.type)
> return -ENOENT;
>
> @@ -11260,6 +11267,13 @@ static void task_clock_event_read(struct perf_event *event)
>
> static int task_clock_event_init(struct perf_event *event)
> {
> + /*
> + * The standard interface to invoke task-clock pmu is through software
> + * pmu(see perf_swevent_init()), since task-clock pmu is not exposed to
> + * the user via sysfs and thus user doesn't know perf_task_clock.type.
> + * However, allow user to open an event if user has passed correct type
> + * in the attribute.
> + */
> if (event->attr.type != perf_task_clock.type)
> return -ENOENT;
>
> --
> 2.34.1
Powered by blists - more mailing lists