Re: Regarding perfmon_capable()

[Leo Yan <leo.yan@linaro.org>]

Hi Anshuman,

On Fri, Jul 01, 2022 at 10:37:37AM +0530, Anshuman Khandual wrote:
> Hello,
> 
> In perf event subsystem and related platform drivers registering a PMU,
> should perfmon_capable() be used directly ? OR just wondering if instead
> perf_allow_[cpu|kernel|tracepoint]() helpers should be used which also
> checks for 'sysctl_perf_event_paranoid' ? Should not both capabilities
> and 'sysctl_perf_event_paranoid' decide whether kernel/cpu/tracepoint
> events will be captured for unprivileged users.

This is an interesting but important topic, let me join the discussion.

Simply to say, sysctl_perf_event_paranoid is a control knob,
perfmon_capable() is for capabilities.  perfmon_capable() only allows
privileged Perf users to access Perf events; on the other hand,
sysctl_perf_event_paranoid can grant green light for non-privileged
users to access perf events.

Therefore, if we use function perf_allow_[cpu|kernel|tracepoint]() as
checking condition which is interfered by sysctl_perf_event_paranoid,
it's superset of perfmon_capable().

On the other hand, even a Perf event can be opened by a non-privileged
process, the low level driver still doesn't want to leak any sensitive
info in the trace data or sampling.  This is why Arm SPE driver checks
the condition perfmon_capable() and disables CONTEXTIDR tracing for
non-privileged users (no matter what's the value of
sysctl_perf_event_paranoid).

Just bear in mind for a corner case, some perf callback functions are
invoked from the kernel threads context rather than user process
context, this is why we might obeserve some strange cases that
non-privileged users might be wrongly granted some tracing
capabilities even we check with perfmon_capable() (Checking
perfmon_capable() is not wrong, but it's wrong to do the checking in
the kernel kthread context rather than user process context).

This is my understanding, just correct me if any thing mentioned
is not reliable.

Thanks,
Leo

> arch/parisc/kernel/perf.c:      if (!perfmon_capable())
> arch/powerpc/perf/imc-pmu.c:    if (!perfmon_capable())
> arch/powerpc/perf/imc-pmu.c:    if (!perfmon_capable())
> drivers/gpu/drm/i915/i915_perf.c:           i915_perf_stream_paranoid && !perfmon_capable()) {
> drivers/gpu/drm/i915/i915_perf.c:                       if (oa_freq_hz > i915_oa_max_sample_rate && !perfmon_capable()) {
> drivers/gpu/drm/i915/i915_perf.c:       if (i915_perf_stream_paranoid && !perfmon_capable()) {
> drivers/gpu/drm/i915/i915_perf.c:       if (i915_perf_stream_paranoid && !perfmon_capable()) {
> drivers/media/rc/bpf-lirc.c:            if (perfmon_capable())
> drivers/perf/arm_spe_pmu.c:     if (IS_ENABLED(CONFIG_PID_IN_CONTEXTIDR) && perfmon_capable())
> drivers/perf/arm_spe_pmu.c:     if (!perfmon_capable() &&
> 
> Although BPF might use perfmon_capabale() alone, because it was never
> dependent on 'sysctl_perf_event_paranoid' ?
> 
> - Anshuman