linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] perf: Use capabilities instead of uid and euid
@ 2019-08-07  3:35 Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 1/4] perf: Add capability-related utilities Igor Lubashev
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Igor Lubashev @ 2019-08-07  3:35 UTC (permalink / raw)
  To: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
	Igor Lubashev

Series v1: https://lkml.kernel.org/lkml/1562112605-6235-1-git-send-email-ilubashe@akamai.com


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 four patches:

  01: perf: Add capability-related utilities
    Add utility functions to check capabilities and perf_event_paranoid checks,
    if libcap-dev[el] is available. (Otherwise, assume no capabilities.)

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

  04: perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
    Replace the use of euid==0 with a check for CAP_SYS_ADMIN before mounting
    debugfs for ftrace.

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.


Changelog:
v2:  * Added a build feature check for libcap-dev[el] as suggested by Arnaldo


Igor Lubashev (4):
  perf: Add capability-related utilities
  perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  perf: Use CAP_SYSLOG with kptr_restrict checks
  perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

 tools/build/Makefile.feature         |  2 ++
 tools/build/feature/Makefile         |  4 ++++
 tools/build/feature/test-libcap.c    | 20 ++++++++++++++++++++
 tools/perf/Makefile.config           | 11 +++++++++++
 tools/perf/Makefile.perf             |  2 ++
 tools/perf/arch/arm/util/cs-etm.c    |  3 ++-
 tools/perf/arch/arm64/util/arm-spe.c |  4 ++--
 tools/perf/arch/x86/util/intel-bts.c |  3 ++-
 tools/perf/arch/x86/util/intel-pt.c  |  2 +-
 tools/perf/builtin-ftrace.c          |  4 +++-
 tools/perf/util/Build                |  2 ++
 tools/perf/util/cap.c                | 29 +++++++++++++++++++++++++++++
 tools/perf/util/cap.h                | 24 ++++++++++++++++++++++++
 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 +++++++++
 18 files changed, 127 insertions(+), 11 deletions(-)
 create mode 100644 tools/build/feature/test-libcap.c
 create mode 100644 tools/perf/util/cap.c
 create mode 100644 tools/perf/util/cap.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/4] perf: Add capability-related utilities
  2019-08-07  3:35 [PATCH v2 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
@ 2019-08-07  3:35 ` Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Igor Lubashev @ 2019-08-07  3:35 UTC (permalink / raw)
  To: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
	Igor Lubashev

Add utilities to help checking capabilities of the running procss.
Make perf link with libcap, if it is available. If no libcap-dev[el],
assume no capabilities.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/build/Makefile.feature       |  2 ++
 tools/build/feature/Makefile       |  4 ++++
 tools/build/feature/test-libcap.c  | 20 ++++++++++++++++++++
 tools/perf/Makefile.config         | 11 +++++++++++
 tools/perf/Makefile.perf           |  2 ++
 tools/perf/util/Build              |  2 ++
 tools/perf/util/cap.c              | 29 +++++++++++++++++++++++++++++
 tools/perf/util/cap.h              | 24 ++++++++++++++++++++++++
 tools/perf/util/event.h            |  1 +
 tools/perf/util/python-ext-sources |  1 +
 tools/perf/util/util.c             |  9 +++++++++
 11 files changed, 105 insertions(+)
 create mode 100644 tools/build/feature/test-libcap.c
 create mode 100644 tools/perf/util/cap.c
 create mode 100644 tools/perf/util/cap.h

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 86b793dffbc4..8a19753cc26a 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -42,6 +42,7 @@ FEATURE_TESTS_BASIC :=                  \
         gtk2-infobar                    \
         libaudit                        \
         libbfd                          \
+        libcap                          \
         libelf                          \
         libelf-getphdrnum               \
         libelf-gelf_getnote             \
@@ -110,6 +111,7 @@ FEATURE_DISPLAY ?=              \
          gtk2                   \
          libaudit               \
          libbfd                 \
+         libcap                 \
          libelf                 \
          libnuma                \
          numa_num_possible_cpus \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 0658b8cd0e53..8499385365c0 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -20,6 +20,7 @@ FILES=                                          \
          test-libbfd-liberty.bin                \
          test-libbfd-liberty-z.bin              \
          test-cplus-demangle.bin                \
+         test-libcap.bin			\
          test-libelf.bin                        \
          test-libelf-getphdrnum.bin             \
          test-libelf-gelf_getnote.bin           \
@@ -105,6 +106,9 @@ $(OUTPUT)test-fortify-source.bin:
 $(OUTPUT)test-bionic.bin:
 	$(BUILD)
 
+$(OUTPUT)test-libcap.bin:
+	$(BUILD) -lcap
+
 $(OUTPUT)test-libelf.bin:
 	$(BUILD) -lelf
 
diff --git a/tools/build/feature/test-libcap.c b/tools/build/feature/test-libcap.c
new file mode 100644
index 000000000000..d2a2e152195f
--- /dev/null
+++ b/tools/build/feature/test-libcap.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <sys/capability.h>
+#include <linux/capability.h>
+
+int main(void)
+{
+	cap_flag_value_t val;
+	cap_t caps = cap_get_proc();
+
+	if (!caps)
+		return 1;
+
+	if (cap_get_flag(caps, CAP_SYS_ADMIN, CAP_EFFECTIVE, &val) != 0)
+		return 1;
+
+	if (cap_free(caps) != 0)
+		return 1;
+
+	return 0;
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index e4988f49ea79..9a06787fedc6 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -824,6 +824,17 @@ ifndef NO_LIBZSTD
   endif
 endif
 
+ifndef NO_LIBCAP
+  ifeq ($(feature-libcap), 1)
+    CFLAGS += -DHAVE_LIBCAP_SUPPORT
+    EXTLIBS += -lcap
+    $(call detected,CONFIG_LIBCAP)
+  else
+    msg := $(warning No libcap found, disables capability support, please install libcap-devel/libcap-dev);
+    NO_LIBCAP := 1
+  endif
+endif
+
 ifndef NO_BACKTRACE
   ifeq ($(feature-backtrace), 1)
     CFLAGS += -DHAVE_BACKTRACE_SUPPORT
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 67512a12276b..f9807d8c005b 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -88,6 +88,8 @@ include ../scripts/utilities.mak
 #
 # Define NO_LIBBPF if you do not want BPF support
 #
+# Define NO_LIBCAP if you do not want process capabilities considered by perf
+#
 # Define NO_SDT if you do not want to define SDT event in perf tools,
 # note that it doesn't disable SDT scanning support.
 #
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 7abf05131889..7cda749059a9 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -148,6 +148,8 @@ perf-$(CONFIG_ZLIB) += zlib.o
 perf-$(CONFIG_LZMA) += lzma.o
 perf-$(CONFIG_ZSTD) += zstd.o
 
+perf-$(CONFIG_LIBCAP) += cap.o
+
 perf-y += demangle-java.o
 perf-y += demangle-rust.o
 
diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
new file mode 100644
index 000000000000..c3ba841bbf37
--- /dev/null
+++ b/tools/perf/util/cap.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Capability utilities
+ */
+
+#ifdef HAVE_LIBCAP_SUPPORT
+
+#include "cap.h"
+#include <stdbool.h>
+#include <sys/capability.h>
+
+bool perf_cap__capable(cap_value_t cap)
+{
+	cap_flag_value_t val;
+	cap_t caps = cap_get_proc();
+
+	if (!caps)
+		return false;
+
+	if (cap_get_flag(caps, cap, CAP_EFFECTIVE, &val) != 0)
+		val = CAP_CLEAR;
+
+	if (cap_free(caps) != 0)
+		return false;
+
+	return val == CAP_SET;
+}
+
+#endif  /* HAVE_LIBCAP_SUPPORT */
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
new file mode 100644
index 000000000000..514b1f8992f6
--- /dev/null
+++ b/tools/perf/util/cap.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_CAP_H
+#define __PERF_CAP_H
+
+#include <stdbool.h>
+#include <linux/capability.h>
+#include <linux/compiler.h>
+
+#ifdef HAVE_LIBCAP_SUPPORT
+
+#include <sys/capability.h>
+
+bool perf_cap__capable(cap_value_t cap);
+
+#else
+
+static inline bool perf_cap__capable(int cap __maybe_unused)
+{
+	return false;
+}
+
+#endif /* HAVE_LIBCAP_SUPPORT */
+
+#endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 70841d115349..0e164e8ae28d 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -851,6 +851,7 @@ void  cpu_map_data__synthesize(struct cpu_map_data *data, struct perf_cpu_map *m
 void event_attr_init(struct perf_event_attr *attr);
 
 int perf_event_paranoid(void);
+bool perf_event_paranoid_check(int max_level);
 
 extern int sysctl_perf_event_max_stack;
 extern int sysctl_perf_event_max_contexts_per_stack;
diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
index 235bd9803390..c6dd478956f1 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -7,6 +7,7 @@
 
 util/python.c
 ../lib/ctype.c
+util/cap.c
 util/evlist.c
 util/evsel.c
 util/cpumap.c
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 9c3c97697387..6fd130a5d8f2 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -16,10 +16,12 @@
 #include <string.h>
 #include <errno.h>
 #include <limits.h>
+#include <linux/capability.h>
 #include <linux/kernel.h>
 #include <linux/log2.h>
 #include <linux/time64.h>
 #include <unistd.h>
+#include "cap.h"
 #include "strlist.h"
 #include "string2.h"
 
@@ -403,6 +405,13 @@ int perf_event_paranoid(void)
 
 	return value;
 }
+
+bool perf_event_paranoid_check(int max_level)
+{
+	return perf_cap__capable(CAP_SYS_ADMIN) ||
+			perf_event_paranoid() <= max_level;
+}
+
 static int
 fetch_ubuntu_kernel_version(unsigned int *puint)
 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-07  3:35 [PATCH v2 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 1/4] perf: Add capability-related utilities Igor Lubashev
@ 2019-08-07  3:35 ` Igor Lubashev
  2019-08-07 11:44   ` Alexey Budankov
  2019-08-07 11:46   ` Jiri Olsa
  2019-08-07  3:35 ` [PATCH v2 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
  3 siblings, 2 replies; 8+ messages in thread
From: Igor Lubashev @ 2019-08-07  3:35 UTC (permalink / raw)
  To: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
	Igor Lubashev

The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
perf_event_paranoid check. Make perf do the same.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/arch/arm/util/cs-etm.c    | 3 ++-
 tools/perf/arch/arm64/util/arm-spe.c | 4 ++--
 tools/perf/arch/x86/util/intel-bts.c | 3 ++-
 tools/perf/arch/x86/util/intel-pt.c  | 2 +-
 tools/perf/util/evsel.c              | 2 +-
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 5cb07e8cb296..b87a1ca2968f 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -18,6 +18,7 @@
 #include "../../perf.h"
 #include "../../util/auxtrace.h"
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evlist.h"
 #include "../../util/evsel.h"
 #include "../../util/pmu.h"
@@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
 	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
 	struct evsel *evsel, *cs_etm_evsel = NULL;
 	struct perf_cpu_map *cpus = evlist->core.cpus;
-	bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+	bool privileged = perf_event_paranoid_check(-1);
 	int err = 0;
 
 	ptr->evlist = evlist;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 00915b8fd05b..200bc973371b 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -12,6 +12,7 @@
 #include <time.h>
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -65,8 +66,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 	struct arm_spe_recording *sper =
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
-	struct evsel *evsel, *arm_spe_evsel = NULL;
-	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+	bool privileged = perf_event_paranoid_check(-1);
 	struct evsel *tracking_evsel;
 	int err;
 
diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
index 7b23318ebd7b..56a76142e9fd 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -12,6 +12,7 @@
 #include <linux/zalloc.h>
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
 	struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
 	struct evsel *evsel, *intel_bts_evsel = NULL;
 	const struct perf_cpu_map *cpus = evlist->core.cpus;
-	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+	bool privileged = perf_event_paranoid_check(-1);
 
 	btsr->evlist = evlist;
 	btsr->snapshot_mode = opts->auxtrace_snapshot_mode;
diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
index 218a4e694618..43d5088ee824 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -558,7 +558,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
 	bool have_timing_info, need_immediate = false;
 	struct evsel *evsel, *intel_pt_evsel = NULL;
 	const struct perf_cpu_map *cpus = evlist->core.cpus;
-	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+	bool privileged = perf_event_paranoid_check(-1);
 	u64 tsc_bit;
 	int err;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 64bc32ed6dfa..eafc134bf17c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -279,7 +279,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 
 static bool perf_event_can_profile_kernel(void)
 {
-	return geteuid() == 0 || perf_event_paranoid() == -1;
+	return perf_event_paranoid_check(-1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
  2019-08-07  3:35 [PATCH v2 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 1/4] perf: Add capability-related utilities Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-07  3:35 ` Igor Lubashev
  2019-08-07  3:35 ` [PATCH v2 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
  3 siblings, 0 replies; 8+ messages in thread
From: Igor Lubashev @ 2019-08-07  3:35 UTC (permalink / raw)
  To: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
	Igor Lubashev

Kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0 when
checking kptr_restrict. Make perf do the same.

Also, the kernel is a more restrictive than "no restrictions" in case of
kptr_restrict==0, so add the same logic to perf.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/util/symbol.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 173f3378aaa0..046271103499 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <linux/capability.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/time64.h>
@@ -15,8 +16,10 @@
 #include <inttypes.h>
 #include "annotate.h"
 #include "build-id.h"
+#include "cap.h"
 #include "util.h"
 #include "debug.h"
+#include "event.h"
 #include "machine.h"
 #include "map.h"
 #include "symbol.h"
@@ -890,7 +893,11 @@ bool symbol__restricted_filename(const char *filename,
 {
 	bool restricted = false;
 
-	if (symbol_conf.kptr_restrict) {
+	/* Per kernel/kallsyms.c:
+	 * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+	 */
+	if (symbol_conf.kptr_restrict ||
+	    (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))) {
 		char *r = realpath(filename, NULL);
 
 		if (r != NULL) {
@@ -2190,9 +2197,9 @@ static bool symbol__read_kptr_restrict(void)
 		char line[8];
 
 		if (fgets(line, sizeof(line), fp) != NULL)
-			value = ((geteuid() != 0) || (getuid() != 0)) ?
-					(atoi(line) != 0) :
-					(atoi(line) == 2);
+			value = perf_cap__capable(CAP_SYSLOG) ?
+					(atoi(line) >= 2) :
+					(atoi(line) != 0);
 
 		fclose(fp);
 	}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
  2019-08-07  3:35 [PATCH v2 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
                   ` (2 preceding siblings ...)
  2019-08-07  3:35 ` [PATCH v2 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
@ 2019-08-07  3:35 ` Igor Lubashev
  3 siblings, 0 replies; 8+ messages in thread
From: Igor Lubashev @ 2019-08-07  3:35 UTC (permalink / raw)
  To: linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
	Igor Lubashev

Kernel requires CAP_SYS_ADMIN instead of euid==0 to mount debugfs for ftrace.
Make perf do the same.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/builtin-ftrace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index ae1466aa3b26..d09eac8a6d57 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -13,6 +13,7 @@
 #include <signal.h>
 #include <fcntl.h>
 #include <poll.h>
+#include <linux/capability.h>
 
 #include "debug.h"
 #include <subcmd/parse-options.h>
@@ -21,6 +22,7 @@
 #include "target.h"
 #include "cpumap.h"
 #include "thread_map.h"
+#include "util/cap.h"
 #include "util/config.h"
 
 
@@ -281,7 +283,7 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
 		.events = POLLIN,
 	};
 
-	if (geteuid() != 0) {
+	if (!perf_cap__capable(CAP_SYS_ADMIN)) {
 		pr_err("ftrace only works for root!\n");
 		return -1;
 	}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-07  3:35 ` [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-07 11:44   ` Alexey Budankov
  2019-08-07 11:46   ` Jiri Olsa
  1 sibling, 0 replies; 8+ messages in thread
From: Alexey Budankov @ 2019-08-07 11:44 UTC (permalink / raw)
  To: Igor Lubashev, linux-kernel, Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris


On 07.08.2019 6:35, Igor Lubashev wrote:
> The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> perf_event_paranoid check. Make perf do the same.
> 
> Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> ---
>  tools/perf/arch/arm/util/cs-etm.c    | 3 ++-
>  tools/perf/arch/arm64/util/arm-spe.c | 4 ++--
>  tools/perf/arch/x86/util/intel-bts.c | 3 ++-
>  tools/perf/arch/x86/util/intel-pt.c  | 2 +-
>  tools/perf/util/evsel.c              | 2 +-
>  5 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index 5cb07e8cb296..b87a1ca2968f 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -18,6 +18,7 @@
>  #include "../../perf.h"
>  #include "../../util/auxtrace.h"
>  #include "../../util/cpumap.h"
> +#include "../../util/event.h"
>  #include "../../util/evlist.h"
>  #include "../../util/evsel.h"
>  #include "../../util/pmu.h"
> @@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
>  	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
>  	struct evsel *evsel, *cs_etm_evsel = NULL;
>  	struct perf_cpu_map *cpus = evlist->core.cpus;
> -	bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
> +	bool privileged = perf_event_paranoid_check(-1);
>  	int err = 0;
>  
>  	ptr->evlist = evlist;
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index 00915b8fd05b..200bc973371b 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -12,6 +12,7 @@
>  #include <time.h>
>  
>  #include "../../util/cpumap.h"
> +#include "../../util/event.h"
>  #include "../../util/evsel.h"
>  #include "../../util/evlist.h"
>  #include "../../util/session.h"
> @@ -65,8 +66,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
>  	struct arm_spe_recording *sper =
>  			container_of(itr, struct arm_spe_recording, itr);
>  	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
> -	struct evsel *evsel, *arm_spe_evsel = NULL;

Makes sense to double check if it compiles with this change.

Regards,
Alexey

> -	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
> +	bool privileged = perf_event_paranoid_check(-1);
>  	struct evsel *tracking_evsel;
>  	int err;
>  
> diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c
> index 7b23318ebd7b..56a76142e9fd 100644
> --- a/tools/perf/arch/x86/util/intel-bts.c
> +++ b/tools/perf/arch/x86/util/intel-bts.c
> @@ -12,6 +12,7 @@
>  #include <linux/zalloc.h>
>  
>  #include "../../util/cpumap.h"
> +#include "../../util/event.h"
>  #include "../../util/evsel.h"
>  #include "../../util/evlist.h"
>  #include "../../util/session.h"
> @@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
>  	struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
>  	struct evsel *evsel, *intel_bts_evsel = NULL;
>  	const struct perf_cpu_map *cpus = evlist->core.cpus;
> -	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
> +	bool privileged = perf_event_paranoid_check(-1);
>  
>  	btsr->evlist = evlist;
>  	btsr->snapshot_mode = opts->auxtrace_snapshot_mode;
> diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c
> index 218a4e694618..43d5088ee824 100644
> --- a/tools/perf/arch/x86/util/intel-pt.c
> +++ b/tools/perf/arch/x86/util/intel-pt.c
> @@ -558,7 +558,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr,
>  	bool have_timing_info, need_immediate = false;
>  	struct evsel *evsel, *intel_pt_evsel = NULL;
>  	const struct perf_cpu_map *cpus = evlist->core.cpus;
> -	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
> +	bool privileged = perf_event_paranoid_check(-1);
>  	u64 tsc_bit;
>  	int err;
>  
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 64bc32ed6dfa..eafc134bf17c 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -279,7 +279,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
>  
>  static bool perf_event_can_profile_kernel(void)
>  {
> -	return geteuid() == 0 || perf_event_paranoid() == -1;
> +	return perf_event_paranoid_check(-1);
>  }
>  
>  struct evsel *perf_evsel__new_cycles(bool precise)
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-07  3:35 ` [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
  2019-08-07 11:44   ` Alexey Budankov
@ 2019-08-07 11:46   ` Jiri Olsa
  2019-08-07 14:56     ` Lubashev, Igor
  1 sibling, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2019-08-07 11:46 UTC (permalink / raw)
  To: Igor Lubashev
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Alexey Budankov,
	Peter Zijlstra, Ingo Molnar, Mathieu Poirier, Alexander Shishkin,
	Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris

On Tue, Aug 06, 2019 at 11:35:55PM -0400, Igor Lubashev wrote:
> The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> perf_event_paranoid check. Make perf do the same.
> 
> Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> ---
>  tools/perf/arch/arm/util/cs-etm.c    | 3 ++-
>  tools/perf/arch/arm64/util/arm-spe.c | 4 ++--
>  tools/perf/arch/x86/util/intel-bts.c | 3 ++-
>  tools/perf/arch/x86/util/intel-pt.c  | 2 +-
>  tools/perf/util/evsel.c              | 2 +-
>  5 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
> index 5cb07e8cb296..b87a1ca2968f 100644
> --- a/tools/perf/arch/arm/util/cs-etm.c
> +++ b/tools/perf/arch/arm/util/cs-etm.c
> @@ -18,6 +18,7 @@
>  #include "../../perf.h"
>  #include "../../util/auxtrace.h"
>  #include "../../util/cpumap.h"
> +#include "../../util/event.h"
>  #include "../../util/evlist.h"
>  #include "../../util/evsel.h"
>  #include "../../util/pmu.h"
> @@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,
>  	struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
>  	struct evsel *evsel, *cs_etm_evsel = NULL;
>  	struct perf_cpu_map *cpus = evlist->core.cpus;
> -	bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
> +	bool privileged = perf_event_paranoid_check(-1);
>  	int err = 0;
>  
>  	ptr->evlist = evlist;
> diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
> index 00915b8fd05b..200bc973371b 100644
> --- a/tools/perf/arch/arm64/util/arm-spe.c
> +++ b/tools/perf/arch/arm64/util/arm-spe.c
> @@ -12,6 +12,7 @@
>  #include <time.h>
>  
>  #include "../../util/cpumap.h"
> +#include "../../util/event.h"
>  #include "../../util/evsel.h"
>  #include "../../util/evlist.h"
>  #include "../../util/session.h"
> @@ -65,8 +66,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
>  	struct arm_spe_recording *sper =
>  			container_of(itr, struct arm_spe_recording, itr);
>  	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
> -	struct evsel *evsel, *arm_spe_evsel = NULL;

wouldn't this removal break the compilation on arm?

jirka

> -	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
> +	bool privileged = perf_event_paranoid_check(-1);
>  	struct evsel *tracking_evsel;
>  	int err;

SNIP

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-07 11:46   ` Jiri Olsa
@ 2019-08-07 14:56     ` Lubashev, Igor
  0 siblings, 0 replies; 8+ messages in thread
From: Lubashev, Igor @ 2019-08-07 14:56 UTC (permalink / raw)
  To: Jiri Olsa, Alexey Budankov
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Mathieu Poirier,
	Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
	Suzuki K Poulose, linux-arm-kernel, James Morris

On Wed, August 7 at 2019 7:46 AM Jiri Olsa wrote:
> On Tue, Aug 06, 2019 at 11:35:55PM -0400, Igor Lubashev wrote:
> > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> > perf_event_paranoid check. Make perf do the same.
> >
> > Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> > ---
> >  tools/perf/arch/arm/util/cs-etm.c    | 3 ++-
> >  tools/perf/arch/arm64/util/arm-spe.c | 4 ++--
> > tools/perf/arch/x86/util/intel-bts.c | 3 ++-
> > tools/perf/arch/x86/util/intel-pt.c  | 2 +-
> >  tools/perf/util/evsel.c              | 2 +-
> >  5 files changed, 8 insertions(+), 6 deletions(-)
> >
SNIP
> > --- a/tools/perf/arch/arm64/util/arm-spe.c
> > +++ b/tools/perf/arch/arm64/util/arm-spe.c
> > @@ -12,6 +12,7 @@
> >  #include <time.h>
> >
> >  #include "../../util/cpumap.h"
> > +#include "../../util/event.h"
> >  #include "../../util/evsel.h"
> >  #include "../../util/evlist.h"
> >  #include "../../util/session.h"
> > @@ -65,8 +66,7 @@ static int arm_spe_recording_options(struct
> auxtrace_record *itr,
> >  	struct arm_spe_recording *sper =
> >  			container_of(itr, struct arm_spe_recording, itr);
> >  	struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
> > -	struct evsel *evsel, *arm_spe_evsel = NULL;
> 
> wouldn't this removal break the compilation on arm?
> 
> jirka
> 
> > -	bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
> > +	bool privileged = perf_event_paranoid_check(-1);
> >  	struct evsel *tracking_evsel;
> >  	int err;
> 
> SNIP

Mea culpa!  (An artifact of a bad rebase.)  Just learned to cross-compile.  Thanks, Alexey and Jirka!

The v3 with the fix has been posted (https://lkml.kernel.org/lkml/cover.1565188228.git.ilubashe@akamai.com).

- Igor

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-08-07 14:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07  3:35 [PATCH v2 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
2019-08-07  3:35 ` [PATCH v2 1/4] perf: Add capability-related utilities Igor Lubashev
2019-08-07  3:35 ` [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
2019-08-07 11:44   ` Alexey Budankov
2019-08-07 11:46   ` Jiri Olsa
2019-08-07 14:56     ` Lubashev, Igor
2019-08-07  3:35 ` [PATCH v2 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
2019-08-07  3:35 ` [PATCH v2 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev

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