Messages in this thread | | | Subject | Re: [RFC PATCH 2/2] perf: arm_spe: Enable CONTEXT packets in SPE traces if the profiler runs in CPU mode. | From | German Gomez <> | Date | Mon, 17 Jan 2022 14:04:22 +0000 |
| |
On 17/01/2022 12:44, German Gomez wrote: > Enable CONTEXT packets in SPE traces if the profiler runs in CPU mode. > This is no less permissive than the existing behavior for the following > reason: > > If perf_event_paranoid <= 0, then non perfmon_capable() users can open > a per-CPU event. With a per-CPU event, unpriviledged users are allowed > to profile _all_ processes, even ones owned by root. > > Without this change, users could see kernel addresses, root processes, > etc, but not gather the PIDs of those processes. The PID is probably the > least sensitive of all the information. > > It would be more idiomatic to check the perf_event_paranoid level with > perf_allow_cpu(), but this function is not exported so cannot be used > from a module. Looking for cpu != -1 is the indirect way of checking > the same thing as it could never get to arm_spe_pmu_event_init() without
Reconsidering this bit (comment below)
> perf_event_paranoid <= 0. > > Co-authored-by: James Clark <james.clark@arm.com> > Signed-off-by: German Gomez <german.gomez@arm.com> > --- > drivers/perf/arm_spe_pmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c > index 8515bf85c..7d9a7fa4f 100644 > --- a/drivers/perf/arm_spe_pmu.c > +++ b/drivers/perf/arm_spe_pmu.c > @@ -711,7 +711,7 @@ static int arm_spe_pmu_event_init(struct perf_event *event) > !(spe_pmu->features & SPE_PMU_FEAT_FILT_LAT)) > return -EOPNOTSUPP; > > - spe_pmu->pmscr_cx = perfmon_capable(); > + spe_pmu->pmscr_cx = perfmon_capable() || (event->cpu != -1);
The perf_event_open(2) manpage states:
pid == -1 and cpu >= 0 This measures all processes/threads on the specified CPU. This requires CAP_PERFMON (since Linux 5.8) or CAP_SYS_ADMIN capability or a /proc/sys/kernel/perf_event_paranoid value of less than 1.
So perhaps it's more accurate to (still implicitly) check the paranoid level with "pid == -1 && event->cpu > 0" ?
If so, I think I have to dig deeper into perf_event. I don't immediately see the pid argument. Any hints?
Thanks, German
> reg = arm_spe_event_to_pmscr(event); > if (!perfmon_capable() && > (reg & (BIT(SYS_PMSCR_EL1_PA_SHIFT) |
| |