lkml.org 
[lkml]   [2020]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v8 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability
    From
    Date

    On 13.07.2020 15:17, Arnaldo Carvalho de Melo wrote:
    > Em Mon, Jul 13, 2020 at 12:48:25PM +0300, Alexey Budankov escreveu:
    >>
    >> On 10.07.2020 20:09, Arnaldo Carvalho de Melo wrote:
    >>> Em Fri, Jul 10, 2020 at 05:30:50PM +0300, Alexey Budankov escreveu:
    >>>> On 10.07.2020 16:31, Ravi Bangoria wrote:
    >>>>>> Currently access to perf_events, i915_perf and other performance
    >>>>>> monitoring and observability subsystems of the kernel is open only for
    >>>>>> a privileged process [1] with CAP_SYS_ADMIN capability enabled in the
    >>>>>> process effective set [2].
    >
    >>>>>> This patch set introduces CAP_PERFMON capability designed to secure
    >>>>>> system performance monitoring and observability operations so that
    >>>>>> CAP_PERFMON would assist CAP_SYS_ADMIN capability in its governing role
    >>>>>> for performance monitoring and observability subsystems of the kernel.
    >
    >>>>> I'm seeing an issue with CAP_PERFMON when I try to record data for a
    >>>>> specific target. I don't know whether this is sort of a regression or
    >>>>> an expected behavior.
    >
    >>>> Thanks for reporting and root causing this case. The behavior looks like
    >>>> kind of expected since currently CAP_PERFMON takes over the related part
    >>>> of CAP_SYS_ADMIN credentials only. Actually Perf security docs [1] say
    >>>> that access control is also subject to CAP_SYS_PTRACE credentials.
    >
    >>> I think that stating that in the error message would be helpful, after
    >>> all, who reads docs? 8-)
    >
    >> At least those who write it :D ...
    >
    > Everybody should read it, sure :-)
    >
    >>> I.e., this:
    >>>
    >>> $ ./perf stat ls
    >>>   Error:
    >>>   Access to performance monitoring and observability operations is limited.
    >>> $
    >>>
    >>> Could become:
    >>>
    >>> $ ./perf stat ls
    >>>   Error:
    >>>   Access to performance monitoring and observability operations is limited.
    >>> Right now only CAP_PERFMON is granted, you may need CAP_SYS_PTRACE.
    >>> $
    >>
    >> It would better provide reference to perf security docs in the tool output.
    >
    > So add a 3rd line:
    >
    > $ ./perf stat ls
    >   Error:
    >   Access to performance monitoring and observability operations is limited.
    > Right now only CAP_PERFMON is granted, you may need CAP_SYS_PTRACE.
    > Please read the 'Perf events and tool security' document:
    > https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
    If it had that patch below then message change would not be required.
    However this two sentences in the end of whole message would still add up:
    "Please read the 'Perf events and tool security' document:
    https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html"

    >
    >> Looks like extending ptrace_may_access() check for perf_events with CAP_PERFMON
    >
    > You mean the following?

    Exactly that.

    >
    > diff --git a/kernel/events/core.c b/kernel/events/core.c
    > index 856d98c36f56..a2397f724c10 100644
    > --- a/kernel/events/core.c
    > +++ b/kernel/events/core.c
    > @@ -11595,7 +11595,7 @@ SYSCALL_DEFINE5(perf_event_open,
    > * perf_event_exit_task() that could imply).
    > */
    > err = -EACCES;
    > - if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS))
    > + if (!perfmon_capable() && !ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS))
    > goto err_cred;
    > }
    >
    >> makes monitoring simpler and even more secure to use since Perf tool need
    >> not to start/stop/single-step and read/write registers and memory and so on
    >> like a debugger or strace-like tool. What do you think?
    >
    > I tend to agree, Peter?
    >
    >> Alexei
    >>
    >>>
    >>> - Arnaldo

    Alexei

    \
     
     \ /
      Last update: 2020-07-13 14:39    [W:5.889 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site