2019-08-07 03:37:47

by Lubashev, Igor

[permalink] [raw]
Subject: [PATCH v2 0/4] perf: Use capabilities instead of uid and euid

Series v1: https://lkml.kernel.org/lkml/[email protected]


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


2019-08-07 03:38:03

by Lubashev, Igor

[permalink] [raw]
Subject: [PATCH v2 1/4] perf: Add capability-related utilities

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 <[email protected]>
---
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

2019-08-07 03:39:38

by Lubashev, Igor

[permalink] [raw]
Subject: [PATCH v2 2/4] perf: 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: Igor Lubashev <[email protected]>
---
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

2019-08-07 03:40:17

by Lubashev, Igor

[permalink] [raw]
Subject: [PATCH v2 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

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

Signed-off-by: Igor Lubashev <[email protected]>
---
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

2019-08-07 11:48:36

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

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 <[email protected]>
> ---
> 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

2019-08-07 12:01:09

by Alexey Budankov

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks


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 <[email protected]>
> ---
> 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)
>

2019-08-07 14:58:13

by Lubashev, Igor

[permalink] [raw]
Subject: RE: [PATCH v2 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

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 <[email protected]>
> > ---
> > 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/[email protected]).

- Igor