linux-kernel - Re: [PATCH v8 00/12] Introduce CAP_PERFMON to secure system performance monitoring and observability

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: <0fadcf78-8b0e-ed03-a554-cc172b7d249c@linux.intel.com>
Date:   Mon, 13 Jul 2020 15:37:51 +0300
From:   Alexey Budankov <alexey.budankov@...ux.intel.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Ravi Bangoria <ravi.bangoria@...ux.ibm.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        James Morris <jmorris@...ei.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Serge Hallyn <serge@...lyn.com>, Jiri Olsa <jolsa@...hat.com>,
        Song Liu <songliubraving@...com>,
        Andi Kleen <ak@...ux.intel.com>,
        Stephane Eranian <eranian@...gle.com>,
        Igor Lubashev <ilubashe@...mai.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        "selinux@...r.kernel.org" <selinux@...r.kernel.org>,
        "intel-gfx@...ts.freedesktop.org" <intel-gfx@...ts.freedesktop.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        linux-man@...r.kernel.org
Subject: Re: [PATCH v8 00/12] Introduce CAP_PERFMON to secure system
 performance monitoring and observability


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