linux-kernel - Re: [PATCH v4 0/9] uprobes: misc cleanups/simplifications

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: <775c414e-03f3-4ae2-80df-9821014e1c32@intel.com>
Date: Fri, 2 Aug 2024 14:02:48 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Peter Zijlstra <peterz@...radead.org>,
 Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Oleg Nesterov <oleg@...hat.com>, andrii@...nel.org, mhiramat@...nel.org,
 jolsa@...nel.org, rostedt@...dmis.org, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH v4 0/9] uprobes: misc cleanups/simplifications

On 2/08/24 12:25, Peter Zijlstra wrote:
> On Thu, Aug 01, 2024 at 02:13:41PM -0700, Andrii Nakryiko wrote:
> 
>> Ok, this bisected to:
>>
>> 675ad74989c2 ("perf/core: Add aux_pause, aux_resume, aux_start_paused")
> 
> Adrian, there are at least two obvious bugs there:
> 
>  - aux_action was key's off of PERF_PMU_CAP_AUX_OUTPUT, which is not
>    right, that's the capability where events can output to AUX -- aka.
>    PEBS-to-PT. It should be PERF_PMU_CAP_ITRACE, which is the
>    PT/CoreSight thing.
> 
>  - it sets aux_paused unconditionally, which is scribbling in the giant
>    union which is overwriting state set by perf_init_event().
> 
> But I think there's more problems, we need to do the aux_action
> validation after perf_get_aux_event(), we can't know if having those
> bits set makes sense before that. This means the perf_event_alloc() site
> is wrong in the first place.
> 
> I'm going to drop these patches for now. Please rework.

Yes I will do that.

FWIW, I'd expect the reported issue would go away with just:

diff --git a/kernel/events/core.c b/kernel/events/core.c
index e4cb6e5a5f40..2072aaa4d449 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -12151,7 +12151,8 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
 		err = -EOPNOTSUPP;
 		goto err_pmu;
 	}
-	event->hw.aux_paused = event->attr.aux_start_paused;
+	if (event->attr.aux_start_paused)
+		event->hw.aux_paused = 1;
 
 	if (cgroup_fd != -1) {
 		err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader);