linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it
@ 2019-08-27  1:39 Igor Lubashev
  2019-08-27  1:39 ` [PATCH 1/5] perf event: Check ref_reloc_sym before using it Igor Lubashev
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

This is a follow up series to the ensure perf treats perf_event_paranoid and
kptr_restrict in a way that is similar to the kernel's.  That includes use of
capabilities instead of euid==0, when possible, as well as adjusting the logic
and fixing bugs.

Prior discussion: https://lkml.kernel.org/lkml/cover.1565188228.git.ilubashe@akamai.com

===  Testing notes ===

I have tested on x86 with perf binary installed according to
Documentation/admin-guide/perf-security.rst (cap_sys_admin, cap_sys_ptrace,
cap_syslog assigned to the perf executable).

I tested each permutation of:

  * 7 commits:
      1. HEAD of perf/core
      2. patch 01 on top of perf/core
      3. patches 01-02 on top of perf/core
      4. patches 01-03 on top of perf/core
      5. patches 01-04 on top of perf/core
      6. patches 01-05 on top of perf/core
      7. HEAD of perf/cap (with known bug fixed by patch 01 of this series)

  * 2 build environments: with and without libcap-dev

  * 3 kernel.kptr_restrict values: 0, 1, 2

  * 4 kernel.perf_event_paranoid values: -1, 0, 1, 2

  * 2 users: root and non-root

Total: 336 permutations

Each permutation consisted of:
  perf test
  perf record -e instructions -- sleep 1

All test runs were expected.  Also, as expected, the following permutation (just
that permutation) resulted in segmentation failure:
 commit:                     perf/cap
 build:                      no libcap-dev
 kernel.kptr_restrict:       0
 kernel.perf_event_paranoid: 2
 user:                       non-root

The perf/cap commit was included in the test to ensure that we can reproduce the
crash and hence test that the patch series fixes the crash, while retaining the
desired behavior of perf/cap.

=== Series Contents ===

  01: perf event: Check ref_reloc_sym before using it
    Fix the pre-existing cause of the crash above: use of ref_reloc_sym without
    a check for NULL

  02: perf tools: 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.
    * This patch has been reviewed previously and is unchanged.
    * I kept Acks and Sign-offs.

  03: perf util: kernel profiling is disallowed only when perf_event_paranoid>1
    Align perf logic regarding perf_event_paranoid to match kernel's.
    This has been reported by Arnaldo.

  04: perf symbols: 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).
    * A previous version of this patch has been reviewed previously, but I
    * modified it in a non-trivial way, so I removed Acks.

  05: perf: warn perf_event_paranoid restrict kernel symbols
    Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
    symbols.

Igor Lubashev (5):
  perf event: Check ref_reloc_sym before using it
  perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
  perf symbols: Use CAP_SYSLOG with kptr_restrict checks
  perf: warn that perf_event_paranoid can restrict kernel symbols

 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/builtin-record.c          |  2 +-
 tools/perf/builtin-top.c             |  2 +-
 tools/perf/builtin-trace.c           |  2 +-
 tools/perf/util/event.c              |  7 ++++---
 tools/perf/util/evsel.c              |  2 +-
 tools/perf/util/symbol.c             | 15 ++++++++++++---
 10 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.7.4


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

* [PATCH 1/5] perf event: Check ref_reloc_sym before using it
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
@ 2019-08-27  1:39 ` Igor Lubashev
  2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
  2019-08-27  1:39 ` [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

Check for ref_reloc_sym before using it instead of checking
symbol_conf.kptr_restrict and relying solely on that check.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 tools/perf/util/event.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index f440fdc3e953..b84f5f3c9651 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -913,11 +913,13 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	int err;
 	union perf_event *event;
 
-	if (symbol_conf.kptr_restrict)
-		return -1;
 	if (map == NULL)
 		return -1;
 
+	kmap = map__kmap(map);
+	if (!kmap->ref_reloc_sym)
+		return -1;
+
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
 	 * available use this, and after it is use this as a fallback for older
@@ -940,7 +942,6 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
 	}
 
-	kmap = map__kmap(map);
 	size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
 			"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
 	size = PERF_ALIGN(size, sizeof(u64));
-- 
2.7.4


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

* [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
  2019-08-27  1:39 ` [PATCH 1/5] perf event: Check ref_reloc_sym before using it Igor Lubashev
@ 2019-08-27  1:39 ` Igor Lubashev
  2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
  2019-08-27  1:39 ` [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1 Igor Lubashev
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

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>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/ad56df5452eeafb99dda9fc3d30f0f487aace503.1565188228.git.ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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/evsel.c              | 2 +-
 5 files changed, 8 insertions(+), 5 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..29275a0544cd 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"
@@ -66,7 +67,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 			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 a8e633aa278a..7abccc0b0dc0 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -578,7 +578,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 0a33f7322ecc..0b3b5af33954 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] 14+ messages in thread

* [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
  2019-08-27  1:39 ` [PATCH 1/5] perf event: Check ref_reloc_sym before using it Igor Lubashev
  2019-08-27  1:39 ` [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-27  1:39 ` Igor Lubashev
  2019-08-27 13:44   ` Arnaldo Carvalho de Melo
  2019-08-29 19:01   ` [tip: perf/core] perf evsel: Kernel " tip-bot2 for Igor Lubashev
  2019-08-27  1:39 ` [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
kernel only disallows profiling when perf_event_paranoid > 1. Make perf do
the same.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0b3b5af33954..bfe6ed2abcc2 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 perf_event_paranoid_check(-1);
+	return perf_event_paranoid_check(1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)
-- 
2.7.4


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

* [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
                   ` (2 preceding siblings ...)
  2019-08-27  1:39 ` [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1 Igor Lubashev
@ 2019-08-27  1:39 ` Igor Lubashev
  2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
  2019-08-27  1:39 ` [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols Igor Lubashev
  2019-08-28 19:31 ` [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Mathieu Poirier
  5 siblings, 1 reply; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

The 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>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/291d2cda6ee75b4cd4c9ce717c177db18bf03a31.1565188228.git.ilubashe@akamai.com
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4efde7879474..035f2e75728c 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"
@@ -2195,13 +2198,19 @@ 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);
 	}
 
+	/* Per kernel/kallsyms.c:
+	 * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+	 */
+	if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+		value = true;
+
 	return value;
 }
 
-- 
2.7.4


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

* [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
                   ` (3 preceding siblings ...)
  2019-08-27  1:39 ` [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
@ 2019-08-27  1:39 ` Igor Lubashev
  2019-08-29 19:01   ` [tip: perf/core] perf tools: Warn " tip-bot2 for Igor Lubashev
  2019-08-28 19:31 ` [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Mathieu Poirier
  5 siblings, 1 reply; 14+ messages in thread
From: Igor Lubashev @ 2019-08-27  1:39 UTC (permalink / raw)
  To: Jiri Olsa, Arnaldo Carvalho de Melo, Mathieu Poirier
  Cc: Igor Lubashev, Alexander Shishkin, Alexey Budankov, James Morris,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Linux Kernel Mailing List

Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
symbols.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/builtin-top.c    | 2 +-
 tools/perf/builtin-trace.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f71631f2bcb5..18505d92ff69 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2372,7 +2372,7 @@ int cmd_record(int argc, const char **argv)
 	if (symbol_conf.kptr_restrict && !perf_evlist__exclude_kernel(rec->evlist))
 		pr_warning(
 "WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
-"check /proc/sys/kernel/kptr_restrict.\n\n"
+"check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 "Samples in kernel functions may not be resolved if a suitable vmlinux\n"
 "file is not found in the buildid cache or in the vmlinux path.\n\n"
 "Samples in kernel modules won't be resolved at all.\n\n"
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5970723cd55a..29e910fb2d9a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -770,7 +770,7 @@ static void perf_event__process_sample(struct perf_tool *tool,
 		if (!perf_evlist__exclude_kernel(top->session->evlist)) {
 			ui__warning(
 "Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
-"Check /proc/sys/kernel/kptr_restrict.\n\n"
+"Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 "Kernel%s samples will not be resolved.\n",
 			  al.map && map__has_symbols(al.map) ?
 			  " modules" : "");
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index bc44ed29e05a..9443b8e05810 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1381,7 +1381,7 @@ static char *trace__machine__resolve_kernel_addr(void *vmachine, unsigned long l
 
 	if (symbol_conf.kptr_restrict) {
 		pr_warning("Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
-			   "Check /proc/sys/kernel/kptr_restrict.\n\n"
+			   "Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 			   "Kernel samples will not be resolved.\n");
 		machine->kptr_restrict_warned = true;
 		return NULL;
-- 
2.7.4


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

* Re: [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
  2019-08-27  1:39 ` [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1 Igor Lubashev
@ 2019-08-27 13:44   ` Arnaldo Carvalho de Melo
  2019-08-29 19:01   ` [tip: perf/core] perf evsel: Kernel " tip-bot2 for Igor Lubashev
  1 sibling, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-27 13:44 UTC (permalink / raw)
  To: Igor Lubashev
  Cc: Jiri Olsa, Mathieu Poirier, Alexander Shishkin, Alexey Budankov,
	James Morris, Namhyung Kim, Peter Zijlstra, Suzuki Poulouse,
	linux-arm-kernel, Linux Kernel Mailing List

Em Mon, Aug 26, 2019 at 09:39:14PM -0400, Igor Lubashev escreveu:
> Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
> kernel only disallows profiling when perf_event_paranoid > 1. Make perf do
> the same.

Thanks for following up on this, I added these notes to this cset commit
log message:

--------------------------------- 8< ------------------------------------

perf evsel: Kernel profiling is disallowed only when perf_event_paranoid > 1

Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
kernel only disallows profiling when perf_event_paranoid > 1. Make perf
do the same.

Committer testing:

For a non-root user:

  $ id
  uid=1000(acme) gid=1000(acme) groups=1000(acme),10(wheel) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
  $

Before:

We were restricting it to just userspace (:u suffix) even for a
workload started by the user:

  $ perf record sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.001 MB perf.data (8 samples) ]
  $ perf evlist
  cycles:u
  $ perf evlist -v
  cycles:u: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, exclude_kernel: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
  $ perf report --stdio
  # To display the perf.data header info, please use --header/--header-only options.
  #
  # Total Lost Samples: 0
  #
  # Samples: 8  of event 'cycles:u'
  # Event count (approx.): 1040396
  #
  # Overhead  Command  Shared Object     Symbol                
  # ........  .......  ................  ......................
  #
      68.36%  sleep    libc-2.29.so      [.] _dl_addr
      27.33%  sleep    ld-2.29.so        [.] dl_main
       3.80%  sleep    ld-2.29.so        [.] _dl_setup_hash
  #
  # (Tip: Order by the overhead of source file name and line number: perf report -s srcline)
  #
  $
  $ 

After:

When the kernel allows profiling the kernel in that scenario:

  $ perf record sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.023 MB perf.data (11 samples) ]
  $ perf evlist
  cycles
  $ perf evlist -v
  cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
  $
  $ perf report --stdio
  # To display the perf.data header info, please use --header/--header-only options.
  #
  # Total Lost Samples: 0
  #
  # Samples: 11  of event 'cycles'
  # Event count (approx.): 1601964
  #
  # Overhead  Command  Shared Object     Symbol                    
  # ........  .......  ................  ..........................
  #
      28.14%  sleep    [kernel.vmlinux]  [k] __rb_erase_color
      27.21%  sleep    [kernel.vmlinux]  [k] unmap_page_range
      27.20%  sleep    ld-2.29.so        [.] __tunable_get_val
      15.24%  sleep    [kernel.vmlinux]  [k] thp_get_unmapped_area
       1.96%  perf     [kernel.vmlinux]  [k] perf_event_exec
       0.22%  perf     [kernel.vmlinux]  [k] native_sched_clock
       0.02%  perf     [kernel.vmlinux]  [k] intel_bts_enable_local
       0.00%  perf     [kernel.vmlinux]  [k] native_write_msr
  #
  # (Tip: Boolean options have negative forms, e.g.: perf report --no-children)
  #
  $

Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-4-git-send-email-ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

# Please enter the commit message for your changes. Lines starting
# with '#' will be ignored, and an empty message aborts the commit.
#
# Author:    Igor Lubashev <ilubashe@akamai.com>
# Date:      Mon Aug 26 21:39:14 2019 -0400
#
# On branch perf/core
# Changes to be committed:
#	modified:   tools/perf/util/evsel.c
#
# Untracked files:
#	a
#	a.c
#	bla
#	f_mode
#	perf.data
#	perf.data.old
#	q
#
# ------------------------ >8 ------------------------
# Do not modify or remove the line above.
# Everything below it will be ignored.
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 7c704b8f0e5c..d4540bfe4574 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -282,7 +282,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 
 static bool perf_event_can_profile_kernel(void)
 {
-	return perf_event_paranoid_check(-1);
+	return perf_event_paranoid_check(1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)

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

* Re: [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it
  2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
                   ` (4 preceding siblings ...)
  2019-08-27  1:39 ` [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols Igor Lubashev
@ 2019-08-28 19:31 ` Mathieu Poirier
  2019-08-28 20:22   ` Arnaldo Carvalho de Melo
  5 siblings, 1 reply; 14+ messages in thread
From: Mathieu Poirier @ 2019-08-28 19:31 UTC (permalink / raw)
  To: Igor Lubashev
  Cc: Jiri Olsa, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Alexey Budankov, James Morris, Namhyung Kim, Peter Zijlstra,
	Suzuki Poulouse, linux-arm-kernel, Linux Kernel Mailing List

On Mon, 26 Aug 2019 at 19:40, Igor Lubashev <ilubashe@akamai.com> wrote:
>
> This is a follow up series to the ensure perf treats perf_event_paranoid and
> kptr_restrict in a way that is similar to the kernel's.  That includes use of
> capabilities instead of euid==0, when possible, as well as adjusting the logic
> and fixing bugs.
>
> Prior discussion: https://lkml.kernel.org/lkml/cover.1565188228.git.ilubashe@akamai.com
>
> ===  Testing notes ===
>
> I have tested on x86 with perf binary installed according to
> Documentation/admin-guide/perf-security.rst (cap_sys_admin, cap_sys_ptrace,
> cap_syslog assigned to the perf executable).
>
> I tested each permutation of:
>
>   * 7 commits:
>       1. HEAD of perf/core
>       2. patch 01 on top of perf/core
>       3. patches 01-02 on top of perf/core
>       4. patches 01-03 on top of perf/core
>       5. patches 01-04 on top of perf/core
>       6. patches 01-05 on top of perf/core
>       7. HEAD of perf/cap (with known bug fixed by patch 01 of this series)
>
>   * 2 build environments: with and without libcap-dev
>
>   * 3 kernel.kptr_restrict values: 0, 1, 2
>
>   * 4 kernel.perf_event_paranoid values: -1, 0, 1, 2
>
>   * 2 users: root and non-root
>
> Total: 336 permutations
>
> Each permutation consisted of:
>   perf test
>   perf record -e instructions -- sleep 1
>
> All test runs were expected.  Also, as expected, the following permutation (just
> that permutation) resulted in segmentation failure:
>  commit:                     perf/cap
>  build:                      no libcap-dev
>  kernel.kptr_restrict:       0
>  kernel.perf_event_paranoid: 2
>  user:                       non-root
>
> The perf/cap commit was included in the test to ensure that we can reproduce the
> crash and hence test that the patch series fixes the crash, while retaining the
> desired behavior of perf/cap.
>
> === Series Contents ===
>
>   01: perf event: Check ref_reloc_sym before using it
>     Fix the pre-existing cause of the crash above: use of ref_reloc_sym without
>     a check for NULL
>
>   02: perf tools: 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.
>     * This patch has been reviewed previously and is unchanged.
>     * I kept Acks and Sign-offs.
>
>   03: perf util: kernel profiling is disallowed only when perf_event_paranoid>1
>     Align perf logic regarding perf_event_paranoid to match kernel's.
>     This has been reported by Arnaldo.
>
>   04: perf symbols: 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).
>     * A previous version of this patch has been reviewed previously, but I
>     * modified it in a non-trivial way, so I removed Acks.
>
>   05: perf: warn perf_event_paranoid restrict kernel symbols
>     Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
>     symbols.
>
> Igor Lubashev (5):
>   perf event: Check ref_reloc_sym before using it
>   perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
>   perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
>   perf symbols: Use CAP_SYSLOG with kptr_restrict checks
>   perf: warn that perf_event_paranoid can restrict kernel symbols
>
>  tools/perf/arch/arm/util/cs-etm.c    |  3 ++-

For the coresight part:

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>  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/builtin-record.c          |  2 +-
>  tools/perf/builtin-top.c             |  2 +-
>  tools/perf/builtin-trace.c           |  2 +-
>  tools/perf/util/event.c              |  7 ++++---
>  tools/perf/util/evsel.c              |  2 +-
>  tools/perf/util/symbol.c             | 15 ++++++++++++---
>  10 files changed, 27 insertions(+), 14 deletions(-)

For the set:

Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

>
> --
> 2.7.4
>

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

* Re: [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it
  2019-08-28 19:31 ` [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Mathieu Poirier
@ 2019-08-28 20:22   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 14+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-28 20:22 UTC (permalink / raw)
  To: Mathieu Poirier
  Cc: Igor Lubashev, Jiri Olsa, Alexander Shishkin, Alexey Budankov,
	James Morris, Namhyung Kim, Peter Zijlstra, Suzuki Poulouse,
	linux-arm-kernel, Linux Kernel Mailing List

Em Wed, Aug 28, 2019 at 01:31:21PM -0600, Mathieu Poirier escreveu:
> On Mon, 26 Aug 2019 at 19:40, Igor Lubashev <ilubashe@akamai.com> wrote:
> > Igor Lubashev (5):
> >   perf event: Check ref_reloc_sym before using it
> >   perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> >   perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
> >   perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> >   perf: warn that perf_event_paranoid can restrict kernel symbols

> >  tools/perf/arch/arm/util/cs-etm.c    |  3 ++-
 
> For the coresight part:
 
> Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
 
> >  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/builtin-record.c          |  2 +-
> >  tools/perf/builtin-top.c             |  2 +-
> >  tools/perf/builtin-trace.c           |  2 +-
> >  tools/perf/util/event.c              |  7 ++++---
> >  tools/perf/util/evsel.c              |  2 +-
> >  tools/perf/util/symbol.c             | 15 ++++++++++++---
> >  10 files changed, 27 insertions(+), 14 deletions(-)
> 
> For the set:
> 
> Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>

Thanks, updated the patches with your tags,

- Arnaldo

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

* [tip: perf/core] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  2019-08-27  1:39 ` [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-29 19:01   ` tip-bot2 for Igor Lubashev
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Igor Lubashev @ 2019-08-29 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Mathieu Poirier,
	Alexander Shishkin, Alexey Budankov, James Morris, Namhyung Kim,
	Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel, Igor Lubashev,
	Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     dda1bf8ea78add78739d128a20b555c4a1a19c27
Gitweb:        https://git.kernel.org/tip/dda1bf8ea78add78739d128a20b555c4a1a19c27
Author:        Igor Lubashev <ilubashe@akamai.com>
AuthorDate:    Mon, 26 Aug 2019 21:39:13 -04:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 28 Aug 2019 17:18:08 -03:00

perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks

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: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org> # coresight part
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-3-git-send-email-ilubashe@akamai.com
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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/evsel.c              | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index a185dab..5d856ed 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -18,6 +18,7 @@
 #include "../../util/record.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 cdd5c0c..c7b38f0 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"
@@ -67,7 +68,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr,
 			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 1f2cf61..16d26ea 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"
@@ -108,7 +109,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 44cfe72..746981c 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -579,7 +579,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 fa67635..7c704b8 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -282,7 +282,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 related	[flat|nested] 14+ messages in thread

* [tip: perf/core] perf event: Check ref_reloc_sym before using it
  2019-08-27  1:39 ` [PATCH 1/5] perf event: Check ref_reloc_sym before using it Igor Lubashev
@ 2019-08-29 19:01   ` tip-bot2 for Igor Lubashev
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Igor Lubashev @ 2019-08-29 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Mathieu Poirier, Igor Lubashev, Alexander Shishkin,
	Alexey Budankov, James Morris, Jiri Olsa, Namhyung Kim,
	Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     e9a6882f267a8105461066e3ea6b4b6b9be1b807
Gitweb:        https://git.kernel.org/tip/e9a6882f267a8105461066e3ea6b4b6b9be1b807
Author:        Igor Lubashev <ilubashe@akamai.com>
AuthorDate:    Mon, 26 Aug 2019 21:39:12 -04:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 28 Aug 2019 17:17:51 -03:00

perf event: Check ref_reloc_sym before using it

Check for ref_reloc_sym before using it instead of checking
symbol_conf.kptr_restrict and relying solely on that check.

Reported-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-2-git-send-email-ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 33616ea..e33dd1a 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -913,11 +913,13 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	int err;
 	union perf_event *event;
 
-	if (symbol_conf.kptr_restrict)
-		return -1;
 	if (map == NULL)
 		return -1;
 
+	kmap = map__kmap(map);
+	if (!kmap->ref_reloc_sym)
+		return -1;
+
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
 	 * available use this, and after it is use this as a fallback for older
@@ -940,7 +942,6 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
 	}
 
-	kmap = map__kmap(map);
 	size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
 			"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
 	size = PERF_ALIGN(size, sizeof(u64));

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

* [tip: perf/core] perf symbols: Use CAP_SYSLOG with kptr_restrict checks
  2019-08-27  1:39 ` [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
@ 2019-08-29 19:01   ` tip-bot2 for Igor Lubashev
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Igor Lubashev @ 2019-08-29 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Igor Lubashev, Mathieu Poirier, Alexander Shishkin,
	Alexey Budankov, James Morris, Jiri Olsa, Namhyung Kim,
	Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     8859aedefefe7eeea5e67968b7fe39c828d589a0
Gitweb:        https://git.kernel.org/tip/8859aedefefe7eeea5e67968b7fe39c828d589a0
Author:        Igor Lubashev <ilubashe@akamai.com>
AuthorDate:    Mon, 26 Aug 2019 21:39:15 -04:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 28 Aug 2019 17:19:19 -03:00

perf symbols: Use CAP_SYSLOG with kptr_restrict checks

The 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>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-5-git-send-email-ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4efde78..035f2e7 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"
@@ -2195,13 +2198,19 @@ 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);
 	}
 
+	/* Per kernel/kallsyms.c:
+	 * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+	 */
+	if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+		value = true;
+
 	return value;
 }
 

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

* [tip: perf/core] perf evsel: Kernel profiling is disallowed only when perf_event_paranoid > 1
  2019-08-27  1:39 ` [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1 Igor Lubashev
  2019-08-27 13:44   ` Arnaldo Carvalho de Melo
@ 2019-08-29 19:01   ` tip-bot2 for Igor Lubashev
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot2 for Igor Lubashev @ 2019-08-29 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnaldo Carvalho de Melo, Igor Lubashev, Mathieu Poirier,
	Alexander Shishkin, Alexey Budankov, James Morris, Jiri Olsa,
	Namhyung Kim, Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Ingo Molnar, Borislav Petkov, linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     aa97293ff129f504e7c8589e56007ecfe3e3e835
Gitweb:        https://git.kernel.org/tip/aa97293ff129f504e7c8589e56007ecfe3e3e835
Author:        Igor Lubashev <ilubashe@akamai.com>
AuthorDate:    Mon, 26 Aug 2019 21:39:14 -04:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 28 Aug 2019 17:19:05 -03:00

perf evsel: Kernel profiling is disallowed only when perf_event_paranoid > 1

Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
kernel only disallows profiling when perf_event_paranoid > 1. Make perf
do the same.

Committer testing:

For a non-root user:

  $ id
  uid=1000(acme) gid=1000(acme) groups=1000(acme),10(wheel) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
  $

Before:

We were restricting it to just userspace (:u suffix) even for a
workload started by the user:

  $ perf record sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.001 MB perf.data (8 samples) ]
  $ perf evlist
  cycles:u
  $ perf evlist -v
  cycles:u: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, exclude_kernel: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
  $ perf report --stdio
  # To display the perf.data header info, please use --header/--header-only options.
  #
  # Total Lost Samples: 0
  #
  # Samples: 8  of event 'cycles:u'
  # Event count (approx.): 1040396
  #
  # Overhead  Command  Shared Object     Symbol
  # ........  .......  ................  ......................
  #
      68.36%  sleep    libc-2.29.so      [.] _dl_addr
      27.33%  sleep    ld-2.29.so        [.] dl_main
       3.80%  sleep    ld-2.29.so        [.] _dl_setup_hash
  #
  # (Tip: Order by the overhead of source file name and line number: perf report -s srcline)
  #
  $
  $

After:

When the kernel allows profiling the kernel in that scenario:

  $ perf record sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.023 MB perf.data (11 samples) ]
  $ perf evlist
  cycles
  $ perf evlist -v
  cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, precise_ip: 3, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1, ksymbol: 1, bpf_event: 1
  $
  $ perf report --stdio
  # To display the perf.data header info, please use --header/--header-only options.
  #
  # Total Lost Samples: 0
  #
  # Samples: 11  of event 'cycles'
  # Event count (approx.): 1601964
  #
  # Overhead  Command  Shared Object     Symbol
  # ........  .......  ................  ..........................
  #
      28.14%  sleep    [kernel.vmlinux]  [k] __rb_erase_color
      27.21%  sleep    [kernel.vmlinux]  [k] unmap_page_range
      27.20%  sleep    ld-2.29.so        [.] __tunable_get_val
      15.24%  sleep    [kernel.vmlinux]  [k] thp_get_unmapped_area
       1.96%  perf     [kernel.vmlinux]  [k] perf_event_exec
       0.22%  perf     [kernel.vmlinux]  [k] native_sched_clock
       0.02%  perf     [kernel.vmlinux]  [k] intel_bts_enable_local
       0.00%  perf     [kernel.vmlinux]  [k] native_write_msr
  #
  # (Tip: Boolean options have negative forms, e.g.: perf report --no-children)
  #
  $

Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-4-git-send-email-ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 7c704b8..d4540bf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -282,7 +282,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 
 static bool perf_event_can_profile_kernel(void)
 {
-	return perf_event_paranoid_check(-1);
+	return perf_event_paranoid_check(1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)

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

* [tip: perf/core] perf tools: Warn that perf_event_paranoid can restrict kernel symbols
  2019-08-27  1:39 ` [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols Igor Lubashev
@ 2019-08-29 19:01   ` tip-bot2 for Igor Lubashev
  0 siblings, 0 replies; 14+ messages in thread
From: tip-bot2 for Igor Lubashev @ 2019-08-29 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Igor Lubashev, Mathieu Poirier, Alexander Shishkin,
	Alexey Budankov, James Morris, Jiri Olsa, Namhyung Kim,
	Peter Zijlstra, Suzuki Poulouse, linux-arm-kernel,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/core branch of tip:

Commit-ID:     d06e5fad8c4692c6e5f1bd626056f23716bfe4a6
Gitweb:        https://git.kernel.org/tip/d06e5fad8c4692c6e5f1bd626056f23716bfe4a6
Author:        Igor Lubashev <ilubashe@akamai.com>
AuthorDate:    Mon, 26 Aug 2019 21:39:16 -04:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 28 Aug 2019 17:19:28 -03:00

perf tools: Warn that perf_event_paranoid can restrict kernel symbols

Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
symbols.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Tested-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Budankov <alexey.budankov@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Suzuki Poulouse <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/1566869956-7154-6-git-send-email-ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/builtin-top.c    | 2 +-
 tools/perf/builtin-trace.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 359bb8f..afe5584 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2372,7 +2372,7 @@ int cmd_record(int argc, const char **argv)
 	if (symbol_conf.kptr_restrict && !perf_evlist__exclude_kernel(rec->evlist))
 		pr_warning(
 "WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,\n"
-"check /proc/sys/kernel/kptr_restrict.\n\n"
+"check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 "Samples in kernel functions may not be resolved if a suitable vmlinux\n"
 "file is not found in the buildid cache or in the vmlinux path.\n\n"
 "Samples in kernel modules won't be resolved at all.\n\n"
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5970723..29e910f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -770,7 +770,7 @@ static void perf_event__process_sample(struct perf_tool *tool,
 		if (!perf_evlist__exclude_kernel(top->session->evlist)) {
 			ui__warning(
 "Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
-"Check /proc/sys/kernel/kptr_restrict.\n\n"
+"Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 "Kernel%s samples will not be resolved.\n",
 			  al.map && map__has_symbols(al.map) ?
 			  " modules" : "");
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 8ea62fd..58a75dd 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1382,7 +1382,7 @@ static char *trace__machine__resolve_kernel_addr(void *vmachine, unsigned long l
 
 	if (symbol_conf.kptr_restrict) {
 		pr_warning("Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
-			   "Check /proc/sys/kernel/kptr_restrict.\n\n"
+			   "Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
 			   "Kernel samples will not be resolved.\n");
 		machine->kptr_restrict_warned = true;
 		return NULL;

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

end of thread, other threads:[~2019-08-29 19:03 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27  1:39 [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Igor Lubashev
2019-08-27  1:39 ` [PATCH 1/5] perf event: Check ref_reloc_sym before using it Igor Lubashev
2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
2019-08-27  1:39 ` [PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
2019-08-27  1:39 ` [PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1 Igor Lubashev
2019-08-27 13:44   ` Arnaldo Carvalho de Melo
2019-08-29 19:01   ` [tip: perf/core] perf evsel: Kernel " tip-bot2 for Igor Lubashev
2019-08-27  1:39 ` [PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
2019-08-29 19:01   ` [tip: perf/core] " tip-bot2 for Igor Lubashev
2019-08-27  1:39 ` [PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols Igor Lubashev
2019-08-29 19:01   ` [tip: perf/core] perf tools: Warn " tip-bot2 for Igor Lubashev
2019-08-28 19:31 ` [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it Mathieu Poirier
2019-08-28 20:22   ` Arnaldo Carvalho de Melo

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