Regarding perfmon_capable()

Regarding perfmon_capable()

[Anshuman Khandual]

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.

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

Re: Regarding perfmon_capable()

[Leo Yan]

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

Re: Regarding perfmon_capable()

[Anshuman Khandual]

On 7/1/22 12:17, Leo Yan wrote:
> 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.

Could not unprivileged users have capabilities too ? I thought that was
the whole point for capabilities.

> 
> 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().

Right. IIUC sysctl_perf_event_paranoid was the original method for perf
event to restrict access, where as capabilities is the new method. Hence
both needs to be checked for compatibility purpose for the original one.

> 
> 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

Can/should low level PMU drivers enforce yet another layer of privilege
check even after the core perf allowed the event to be created in the
first place ?

> 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).

Is not pmu->event_init() called in user process context itself. Why can
not all privillege checking be done there and stored (if required) some
where more platform specific e.g event->hw.config or any other platform
data structure. Why should privilege gets checked in callbacks which
might run in privilege contexts to create such corner cases ?

> 
> 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

Re: Regarding perfmon_capable()

[Leo Yan]

On Fri, Jul 01, 2022 at 12:52:36PM +0530, Anshuman Khandual wrote:
> 
> 
> On 7/1/22 12:17, Leo Yan wrote:
> > 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.
> 
> Could not unprivileged users have capabilities too ? I thought that was
> the whole point for capabilities.

Agree, I might introduce confusion so just clarify.

Based on the document Documentation/admin-guide/perf-security.rst, we can
"create perf_users group for privileged Perf users".  Here privileged
users mean not only the root user, it also includes the users in
perf_users group.

> > 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().
> 
> Right. IIUC sysctl_perf_event_paranoid was the original method for perf
> event to restrict access, where as capabilities is the new method. Hence
> both needs to be checked for compatibility purpose for the original one.
> 
> > 
> > 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
> 
> Can/should low level PMU drivers enforce yet another layer of privilege
> check even after the core perf allowed the event to be created in the
> first place ?

Good point.  Seems to me, Perf core layer checks permission with a big
granularity, and low level driver can control permission with smaller
granularity, e.g. for a perf event, the low level driver can control
only its partial functionality used by non-privileged users.

> > 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).
> 
> Is not pmu->event_init() called in user process context itself. Why can
> not all privillege checking be done there and stored (if required) some
> where more platform specific e.g event->hw.config or any other platform
> data structure. Why should privilege gets checked in callbacks which
> might run in privilege contexts to create such corner cases ?

TBH, I didn't remember in details, but let's assume if Perf works in
system wide mode and it needs to enable PMU for all CPUs in the system.
IIUC, it needs to use per CPU's workqueue to enable PMU events, I think
the function event_function_call() can give hints for why some Perf flow
is not invoked in user process context.

Thanks,
Leo