linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Alexey Budankov <alexey.budankov@linux.intel.com>
To: Igor Lubashev <ilubashe@akamai.com>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Kees Cook <keescook@chromium.org>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Alexey Budankov <alexey.budankov@linux.intel.com>,
	James Morris <jmorris@namei.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jiri Olsa <jolsa@redhat.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 0/3] perf: Use capabilities instead of uid and euid
Date: Tue, 16 Jul 2019 13:51:06 +0300	[thread overview]
Message-ID: <cf6398f2-8862-9062-8765-80f930c019e2@linux.intel.com> (raw)
In-Reply-To: <1562112605-6235-1-git-send-email-ilubashe@akamai.com>

On 03.07.2019 3:10, Igor Lubashev wrote:
> Kernel is using capabilities instead of uid and euid to restrict access to
> kernel pointers and tracing facilities.  This patch series updates the perf to
> better match the security model used by the kernel.
> 
> This series enables instructions in Documentation/admin-guide/perf-security.rst
> to actually work, even when kernel.perf_event_paranoid=2 and
> kernel.kptr_restrict=1.
> 
> The series consists of three patches:
> 
>   01: perf: Add capability-related utilities
>     Add utility functions to check capabilities and perf_event_paranoid checks.
> 
>   02: perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
>     Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
>     perf_event_paranoid level is verified.
> 
>   03: perf: Use CAP_SYSLOG with kptr_restrict checks
>     Replace the use of uid and euid with a check for CAP_SYSLOG when
>     kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
>     Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).
> 
> I tested this by following Documentation/admin-guide/perf-security.rst
> guidelines and setting sysctls:
> 
>    kernel.perf_event_paranoid=2
>    kernel.kptr_restrict=1
> 
> As an unpriviledged user who is in perf_users group (setup via instructions
> above), I executed:
>    perf record -a -- sleep 1
> 
> Without the patch, perf record did not capture any kernel functions.
> With the patch, perf included all kernel funcitons.

Acked-by: Alexey Budankov <alexey.budankov@linux.intel.com>

Valuable contribution, thanks! And I see the continuation of the effort started 
in this patch set. Some dedicated CAP_SYS_PERFMON capability could be introduced 
and used for performance monitoring related security checks, as in the kernel as 
in the user mode, because CAP_SYS_ADMIN grants much wider credentials that are 
required, at least for Perf related monitoring and, yet more, CAP_SYS_ADMIN could 
be unloaded addressing the concerns here [1]:

 CAP_SYS_ADMIN
       	   Note: this capability is overloaded; see Notes to kernel developers, below.
 ...
 Notes to kernel developers:
	   When adding a new kernel feature that should be governed by a
	   capability, consider the following points.
	   *  The goal of capabilities is divide the power of superuser into
	       pieces, such that if a program that has one or more capabilities
	       is compromised, its power to do damage to the system would be less
	       than the same program running with root privilege.
	   *  You have the choice of either creating a new capability for your
	       new feature, or associating the feature with one of the existing
	       capabilities.  In order to keep the set of capabilities to a
	       manageable size, the latter option is preferable, unless there are
	       compelling reasons to take the former option.  (There is also a
	       technical limit: the size of capability sets is currently limited
	       to 64 bits.)
	       . . .
	    * Don't choose CAP_SYS_ADMIN if you can possibly avoid it!  A vast
	       proportion of existing capability checks are associated with this
	       capability (see the partial list above).  It can plausibly be
	       called "the new root", since on the one hand, it confers a wide
	       range of powers, and on the other hand, its broad scope means that
	       this is the capability that is required by many privileged
	       programs.  Don't make the problem worse.  The only new features
	       that should be associated with CAP_SYS_ADMIN are ones that closely
	       match existing uses in that silo.
	    * If you have determined that it really is necessary to create a new
	       capability for your feature, don't make or name it as a "single-
	       use" capability.  Thus, for example, the addition of the highly
	       specific CAP_SYS_PACCT was probably a mistake.  Instead, try to
	       identify and name your new capability as a broader silo into which
           other related future use cases might fit.”

Regards,
Alexey

[1] http://man7.org/linux/man-pages/man7/capabilities.7.html

> 
> Igor Lubashev (3):
>   perf: Add capability-related utilities
>   perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
>   perf: Use CAP_SYSLOG with kptr_restrict checks
> 
>  tools/perf/Makefile.config           |  2 +-
>  tools/perf/arch/arm/util/cs-etm.c    |  3 ++-
>  tools/perf/arch/arm64/util/arm-spe.c |  3 ++-
>  tools/perf/arch/x86/util/intel-bts.c |  3 ++-
>  tools/perf/arch/x86/util/intel-pt.c  |  2 +-
>  tools/perf/util/Build                |  1 +
>  tools/perf/util/cap.c                | 24 ++++++++++++++++++++++++
>  tools/perf/util/cap.h                | 10 ++++++++++
>  tools/perf/util/event.h              |  1 +
>  tools/perf/util/evsel.c              |  2 +-
>  tools/perf/util/python-ext-sources   |  1 +
>  tools/perf/util/symbol.c             | 15 +++++++++++----
>  tools/perf/util/util.c               |  9 +++++++++
>  13 files changed, 66 insertions(+), 10 deletions(-)
>  create mode 100644 tools/perf/util/cap.c
>  create mode 100644 tools/perf/util/cap.h
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

      parent reply	other threads:[~2019-07-16 10:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03  0:10 [PATCH 0/3] perf: Use capabilities instead of uid and euid Igor Lubashev
2019-07-03  0:10 ` [PATCH 1/3] perf: Add capability-related utilities Igor Lubashev
2019-07-16  8:46   ` Jiri Olsa
2019-07-17 21:05     ` Arnaldo Carvalho de Melo
2019-07-17 23:46       ` Arnaldo Carvalho de Melo
2019-07-17 23:48         ` Arnaldo Carvalho de Melo
2019-07-18 21:00         ` Lubashev, Igor
2019-08-07  3:58         ` Lubashev, Igor
2019-07-03  0:10 ` [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
2019-07-16  8:47   ` Jiri Olsa
2019-07-16 17:01     ` Lubashev, Igor
2019-07-17  7:10       ` Jiri Olsa
2019-07-17 18:33         ` Lubashev, Igor
2019-07-03  0:10 ` [PATCH 3/3] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
2019-07-16 10:51 ` Alexey Budankov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf6398f2-8862-9062-8765-80f930c019e2@linux.intel.com \
    --to=alexey.budankov@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=ilubashe@akamai.com \
    --cc=jmorris@namei.org \
    --cc=jolsa@redhat.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).