* [PATCH v3 1/4] perf: Add capability-related utilities
2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
@ 2019-08-07 14:44 ` Igor Lubashev
2019-08-12 19:43 ` Arnaldo Carvalho de Melo
` (2 more replies)
2019-08-07 14:44 ` [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
` (3 subsequent siblings)
4 siblings, 3 replies; 33+ messages in thread
From: Igor Lubashev @ 2019-08-07 14:44 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] 33+ messages in thread
* Re: [PATCH v3 1/4] perf: Add capability-related utilities
2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev
@ 2019-08-12 19:43 ` Arnaldo Carvalho de Melo
2019-08-15 9:24 ` [tip:perf/core] tools build: Add capability-related feature detection tip-bot for Igor Lubashev
2019-08-15 9:25 ` [tip:perf/core] perf tools: Add helpers to use capabilities if present tip-bot for Igor Lubashev
2 siblings, 0 replies; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 19:43 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Wed, Aug 07, 2019 at 10:44:14AM -0400, Igor Lubashev escreveu:
> 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.
Applied and added the text at the end of this message on the commit log,
I'm also adding NO_LIBCAP=1 to the minimum build in
make -C tools/perf build-test
I.e.:
diff --git a/tools/perf/tests/make b/tools/perf/tests/make
index 5363a12a8b9b..70c48475896d 100644
--- a/tools/perf/tests/make
+++ b/tools/perf/tests/make
@@ -108,6 +108,7 @@ make_minimal += NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1
make_minimal += NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1
make_minimal += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1
make_minimal += NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1 NO_LIBZSTD=1
+make_minimal += NO_LIBCAP=1
# $(run) contains all available tests
run := make_pure
Thanks,
- Arnaldo
Committer testing:
$ make O=/tmp/build/perf -C tools/perf install-bin
make: Entering directory '/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j8' parallel build
Auto-detecting system features:
<SNIP>
... libbfd: [ on ]
... libcap: [ OFF ]
... libelf: [ on ]
<SNIP>
Makefile.config:833: No libcap found, disables capability support, please install libcap-devel/libcap-dev
<SNIP>
$ grep libcap /tmp/build/perf/FEATURE-DUMP
feature-libcap=0
$ cat /tmp/build/perf/feature/test-libcap.make.output
test-libcap.c:2:10: fatal error: sys/capability.h: No such file or directory
2 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
$
Now install libcap-devel and try again:
$ make O=/tmp/build/perf -C tools/perf install-bin
make: Entering directory '/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j8' parallel build
Warning: Kernel ABI header at 'tools/include/linux/bits.h' differs from latest version at 'include/linux/bits.h'
diff -u tools/include/linux/bits.h include/linux/bits.h
Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
... gtk2: [ on ]
... libaudit: [ on ]
... libbfd: [ on ]
... libcap: [ on ]
... libelf: [ on ]
<SNIP>>
CC /tmp/build/perf/jvmti/libjvmti.o
<SNIP>>
$ grep libcap /tmp/build/perf/FEATURE-DUMP
feature-libcap=1
$ cat /tmp/build/perf/feature/test-libcap.make.output
$ ldd /tmp/build/perf/feature/test-libcap.make.bin
ldd: /tmp/build/perf/feature/test-libcap.make.bin: No such file or directory
$ ldd /tmp/build/perf/feature/test-libcap.bin
linux-vdso.so.1 (0x00007ffc35bfe000)
libcap.so.2 => /lib64/libcap.so.2 (0x00007ff9c62ff000)
libc.so.6 => /lib64/libc.so.6 (0x00007ff9c6139000)
/lib64/ld-linux-x86-64.so.2 (0x00007ff9c6326000)
$
$ ldd ~/bin/perf | grep libcap
libcap.so.2 => /lib64/libcap.so.2 (0x00007fe20ea19000)
$
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [tip:perf/core] tools build: Add capability-related feature detection
2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev
2019-08-12 19:43 ` Arnaldo Carvalho de Melo
@ 2019-08-15 9:24 ` tip-bot for Igor Lubashev
2019-08-15 9:25 ` [tip:perf/core] perf tools: Add helpers to use capabilities if present tip-bot for Igor Lubashev
2 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Igor Lubashev @ 2019-08-15 9:24 UTC (permalink / raw)
To: linux-tip-commits
Cc: ilubashe, mathieu.poirier, tglx, suzuki.poulose, peterz, mingo,
alexey.budankov, alexander.shishkin, linux-kernel, hpa, jolsa,
namhyung, jmorris, acme
Commit-ID: 74d5f3d06f707eb5f7e1908ad88954bde02000ce
Gitweb: https://git.kernel.org/tip/74d5f3d06f707eb5f7e1908ad88954bde02000ce
Author: Igor Lubashev <ilubashe@akamai.com>
AuthorDate: Wed, 7 Aug 2019 10:44:14 -0400
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 12 Aug 2019 17:14:14 -0300
tools build: Add capability-related feature detection
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.
Committer testing:
$ make O=/tmp/build/perf -C tools/perf install-bin
make: Entering directory '/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j8' parallel build
Auto-detecting system features:
<SNIP>
... libbfd: [ on ]
... libcap: [ OFF ]
... libelf: [ on ]
<SNIP>
Makefile.config:833: No libcap found, disables capability support, please install libcap-devel/libcap-dev
<SNIP>
$ grep libcap /tmp/build/perf/FEATURE-DUMP
feature-libcap=0
$ cat /tmp/build/perf/feature/test-libcap.make.output
test-libcap.c:2:10: fatal error: sys/capability.h: No such file or directory
2 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
$
Now install libcap-devel and try again:
$ make O=/tmp/build/perf -C tools/perf install-bin
make: Entering directory '/home/acme/git/perf/tools/perf'
BUILD: Doing 'make -j8' parallel build
Warning: Kernel ABI header at 'tools/include/linux/bits.h' differs from latest version at 'include/linux/bits.h'
diff -u tools/include/linux/bits.h include/linux/bits.h
Warning: Kernel ABI header at 'tools/arch/x86/include/asm/cpufeatures.h' differs from latest version at 'arch/x86/include/asm/cpufeatures.h'
diff -u tools/arch/x86/include/asm/cpufeatures.h arch/x86/include/asm/cpufeatures.h
Auto-detecting system features:
<SNIP>
... libbfd: [ on ]
... libcap: [ on ]
... libelf: [ on ]
<SNIP>>
CC /tmp/build/perf/jvmti/libjvmti.o
<SNIP>>
$ grep libcap /tmp/build/perf/FEATURE-DUMP
feature-libcap=1
$ cat /tmp/build/perf/feature/test-libcap.make.output
$ ldd /tmp/build/perf/feature/test-libcap.make.bin
ldd: /tmp/build/perf/feature/test-libcap.make.bin: No such file or directory
$ ldd /tmp/build/perf/feature/test-libcap.bin
linux-vdso.so.1 (0x00007ffc35bfe000)
libcap.so.2 => /lib64/libcap.so.2 (0x00007ff9c62ff000)
libc.so.6 => /lib64/libc.so.6 (0x00007ff9c6139000)
/lib64/ld-linux-x86-64.so.2 (0x00007ff9c6326000)
$
Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
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: 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>
[ split from a larger patch ]
Link: http://lkml.kernel.org/r/8a1e76cf5c7c9796d0d4d240fbaa85305298aafa.1565188228.git.ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 ++
5 files changed, 39 insertions(+)
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.
#
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [tip:perf/core] perf tools: Add helpers to use capabilities if present
2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev
2019-08-12 19:43 ` Arnaldo Carvalho de Melo
2019-08-15 9:24 ` [tip:perf/core] tools build: Add capability-related feature detection tip-bot for Igor Lubashev
@ 2019-08-15 9:25 ` tip-bot for Igor Lubashev
2 siblings, 0 replies; 33+ messages in thread
From: tip-bot for Igor Lubashev @ 2019-08-15 9:25 UTC (permalink / raw)
To: linux-tip-commits
Cc: alexander.shishkin, tglx, hpa, jolsa, mathieu.poirier,
linux-kernel, peterz, alexey.budankov, jmorris, mingo, namhyung,
ilubashe, suzuki.poulose, acme
Commit-ID: c22e150e3afa6f8db2300bd510e4ac26bbee1bf3
Gitweb: https://git.kernel.org/tip/c22e150e3afa6f8db2300bd510e4ac26bbee1bf3
Author: Igor Lubashev <ilubashe@akamai.com>
AuthorDate: Wed, 7 Aug 2019 10:44:14 -0400
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 14 Aug 2019 10:48:39 -0300
perf tools: Add helpers to use capabilities if present
Add utilities to help checking capabilities of the running procss. Make
perf link with libcap, if it is available. If no libcap-dev[el],
fallback to the geteuid() == 0 test used before.
Committer notes:
$ perf test python
18: 'import perf' in python : FAILED!
$ perf test -v python
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
18: 'import perf' in python :
--- start ---
test child forked, pid 23288
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: /tmp/build/perf/python/perf.so: undefined symbol: cap_get_flag
test child finished with -1
---- end ----
'import perf' in python: FAILED!
$
This happens because differently from the perf binary generated with
this patch applied:
$ ldd /tmp/build/perf/perf | grep libcap
libcap.so.2 => /lib64/libcap.so.2 (0x00007f724a4ef000)
$
The python binding isn't linking with libcap:
$ ldd /tmp/build/perf/python/perf.so | grep libcap
$
So add 'cap' to the 'extra_libraries' variable in
tools/perf/util/setup.py, and rebuild:
$ perf test python
18: 'import perf' in python : Ok
$
If we explicitely disable libcap it also continues to work:
$ make NO_LIBCAP=1 -C tools/perf O=/tmp/build/perf install-bin
$ ldd /tmp/build/perf/perf | grep libcap
$ ldd /tmp/build/perf/python/perf.so | grep libcap
$ perf test python
18: 'import perf' in python : Ok
$
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
[ split from a larger patch ]
Link: http://lkml.kernel.org/r/8a1e76cf5c7c9796d0d4d240fbaa85305298aafa.1565188228.git.ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/Build | 2 ++
tools/perf/util/cap.c | 29 +++++++++++++++++++++++++++++
tools/perf/util/cap.h | 27 +++++++++++++++++++++++++++
tools/perf/util/event.h | 1 +
tools/perf/util/python-ext-sources | 1 +
tools/perf/util/setup.py | 2 ++
tools/perf/util/util.c | 9 +++++++++
7 files changed, 71 insertions(+)
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..10af94e473da
--- /dev/null
+++ b/tools/perf/util/cap.h
@@ -0,0 +1,27 @@
+/* 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
+
+#include <unistd.h>
+#include <sys/types.h>
+
+static inline bool perf_cap__capable(int cap __maybe_unused)
+{
+ return geteuid() == 0;
+}
+
+#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/setup.py b/tools/perf/util/setup.py
index d48f9cd58964..aa344a163eaf 100644
--- a/tools/perf/util/setup.py
+++ b/tools/perf/util/setup.py
@@ -59,6 +59,8 @@ ext_sources = list(map(lambda x: '%s/%s' % (src_perf, x) , ext_sources))
extra_libraries = []
if '-DHAVE_LIBNUMA_SUPPORT' in cflags:
extra_libraries = [ 'numa' ]
+if '-DHAVE_LIBCAP_SUPPORT' in cflags:
+ extra_libraries += [ 'cap' ]
perf = Extension('perf',
sources = ext_sources,
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)
{
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev
@ 2019-08-07 14:44 ` Igor Lubashev
2019-08-12 20:01 ` Arnaldo Carvalho de Melo
2019-08-07 14:44 ` [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
` (2 subsequent siblings)
4 siblings, 1 reply; 33+ messages in thread
From: Igor Lubashev @ 2019-08-07 14:44 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 | 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 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] 33+ messages in thread
* Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
2019-08-07 14:44 ` [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-12 20:01 ` Arnaldo Carvalho de Melo
2019-08-12 20:15 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 20:01 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu:
> +++ 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);
> }
While looking at your changes I think the pre-existing code is wrong,
i.e. the check in sys_perf_event_open(), in the kernel is:
if (!attr.exclude_kernel) {
if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
return -EACCES;
}
And:
static inline bool perf_paranoid_kernel(void)
{
return sysctl_perf_event_paranoid > 1;
}
So we have to change that perf_event_paranoit_check(-1) to pass 1
instead?
bool perf_event_paranoid_check(int max_level)
{
return perf_cap__capable(CAP_SYS_ADMIN) ||
perf_event_paranoid() <= max_level;
}
Also you defined perf_cap__capable(anything) as:
#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 */
I think we should have:
#else
static inline bool perf_cap__capable(int cap __maybe_unused)
{
return geteuid() == 0;
}
#endif /* HAVE_LIBCAP_SUPPORT */
Right?
So I am removing the introduction of perf_cap__capable() from the first
patch you sent, leaving it with _only_ the feature detection part, using
that feature detection to do anything is then moved to a separate patch,
after we finish this discussion about what we should fallback to when
libcap-devel isn't available, i.e. we should use the previous checks,
etc.
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
2019-08-12 20:01 ` Arnaldo Carvalho de Melo
@ 2019-08-12 20:15 ` Arnaldo Carvalho de Melo
2019-08-12 22:33 ` Lubashev, Igor
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 20:15 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Mon, Aug 12, 2019 at 05:01:34PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu:
> > +++ 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);
> > }
>
> While looking at your changes I think the pre-existing code is wrong,
> i.e. the check in sys_perf_event_open(), in the kernel is:
>
> if (!attr.exclude_kernel) {
> if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> return -EACCES;
> }
>
> And:
>
> static inline bool perf_paranoid_kernel(void)
> {
> return sysctl_perf_event_paranoid > 1;
> }
>
> So we have to change that perf_event_paranoit_check(-1) to pass 1
> instead?
>
> bool perf_event_paranoid_check(int max_level)
> {
> return perf_cap__capable(CAP_SYS_ADMIN) ||
> perf_event_paranoid() <= max_level;
> }
>
> Also you defined perf_cap__capable(anything) as:
>
> #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 */
>
>
> I think we should have:
>
> #else
>
> static inline bool perf_cap__capable(int cap __maybe_unused)
> {
> return geteuid() == 0;
> }
>
> #endif /* HAVE_LIBCAP_SUPPORT */
>
> Right?
>
> So I am removing the introduction of perf_cap__capable() from the first
> patch you sent, leaving it with _only_ the feature detection part, using
> that feature detection to do anything is then moved to a separate patch,
> after we finish this discussion about what we should fallback to when
> libcap-devel isn't available, i.e. we should use the previous checks,
> etc.
So, please take a look at the tmp.perf/cap branch in my git repo:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.perf/cap
I split the patch and made perf_cap__capable() fallback to 'return
geteuid() == 0;' when libcap-devel isn't available, i.e. keep the
checks made prior to your patchset.
Jiri, can I keep your Acked-by?
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
2019-08-12 20:15 ` Arnaldo Carvalho de Melo
@ 2019-08-12 22:33 ` Lubashev, Igor
2019-08-13 13:20 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Lubashev, Igor @ 2019-08-12 22:33 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
On Mon, August 12, 2019 at 4:16 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> Em Mon, Aug 12, 2019 at 05:01:34PM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu:
> > > +++ 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);
> > > }
> >
> > While looking at your changes I think the pre-existing code is wrong,
> > i.e. the check in sys_perf_event_open(), in the kernel is:
> >
> > if (!attr.exclude_kernel) {
> > if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > return -EACCES;
> > }
> >
> > And:
> >
> > static inline bool perf_paranoid_kernel(void) {
> > return sysctl_perf_event_paranoid > 1; }
> >
> > So we have to change that perf_event_paranoit_check(-1) to pass 1
> > instead?
Indeed. This seems right. It was a pre-existing problem.
> > bool perf_event_paranoid_check(int max_level) {
> > return perf_cap__capable(CAP_SYS_ADMIN) ||
> > perf_event_paranoid() <= max_level; }
> >
> > Also you defined perf_cap__capable(anything) as:
> >
> > #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 */
> >
> >
> > I think we should have:
> >
> > #else
> >
> > static inline bool perf_cap__capable(int cap __maybe_unused) {
> > return geteuid() == 0;
> > }
> >
> > #endif /* HAVE_LIBCAP_SUPPORT */
> >
> > Right?
You can have EUID==0 and not have CAP_SYS_ADMIN, though this would be rare in practice. I did not to use EUID in leu of libcap, since kernel does not do so, and therefore it seemed a bit misleading. But this is a slight matter of taste, and I do not see a problem with choosing to fall back to EUID -- the kernel will do the right thing anyway.
Now, if I were pedantic, I'd say that to use geteuid(), you need to #include <unistd.h> .
> > So I am removing the introduction of perf_cap__capable() from the
> > first patch you sent, leaving it with _only_ the feature detection
> > part, using that feature detection to do anything is then moved to a
> > separate patch, after we finish this discussion about what we should
> > fallback to when libcap-devel isn't available, i.e. we should use the
> > previous checks, etc.
>
> So, please take a look at the tmp.perf/cap branch in my git repo:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.p
> erf/cap
>
> I split the patch and made perf_cap__capable() fallback to 'return
> geteuid() == 0;' when libcap-devel isn't available, i.e. keep the checks made
> prior to your patchset.
Thank you. And thanks for updating "make_minimal".
>
> Jiri, can I keep your Acked-by?
>
> - Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
2019-08-12 22:33 ` Lubashev, Igor
@ 2019-08-13 13:20 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-13 13:20 UTC (permalink / raw)
To: Lubashev, Igor
Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Mathieu Poirier,
Alexander Shishkin, Namhyung Kim, Suzuki K Poulose,
linux-arm-kernel, James Morris
Em Mon, Aug 12, 2019 at 10:33:07PM +0000, Lubashev, Igor escreveu:
> On Mon, August 12, 2019 at 4:16 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Em Mon, Aug 12, 2019 at 05:01:34PM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu:
> > > > +++ 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);
> > > > }
> > >
> > > While looking at your changes I think the pre-existing code is wrong,
> > > i.e. the check in sys_perf_event_open(), in the kernel is:
> > >
> > > if (!attr.exclude_kernel) {
> > > if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
> > > return -EACCES;
> > > }
> > >
> > > And:
> > >
> > > static inline bool perf_paranoid_kernel(void) {
> > > return sysctl_perf_event_paranoid > 1; }
> > >
> > > So we have to change that perf_event_paranoit_check(-1) to pass 1
> > > instead?
>
> Indeed. This seems right. It was a pre-existing problem.
>
>
> > > bool perf_event_paranoid_check(int max_level) {
> > > return perf_cap__capable(CAP_SYS_ADMIN) ||
> > > perf_event_paranoid() <= max_level; }
> > >
> > > Also you defined perf_cap__capable(anything) as:
> > >
> > > #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 */
> > >
> > >
> > > I think we should have:
> > >
> > > #else
> > >
> > > static inline bool perf_cap__capable(int cap __maybe_unused) {
> > > return geteuid() == 0;
> > > }
> > >
> > > #endif /* HAVE_LIBCAP_SUPPORT */
> > >
> > > Right?
>
> You can have EUID==0 and not have CAP_SYS_ADMIN, though this would be rare in practice. I did not to use EUID in leu of libcap, since kernel does not do so, and therefore it seemed a bit misleading. But this is a slight matter of taste, and I do not see a problem with choosing to fall back to EUID -- the kernel will do the right thing anyway.
>
> Now, if I were pedantic, I'd say that to use geteuid(), you need to #include <unistd.h> .
Right, and that is how I did it :-)
[acme@seventh perf]$ cat tools/perf/util/cap.h
/* 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
#include <unistd.h>
#include <sys/types.h>
static inline bool perf_cap__capable(int cap __maybe_unused)
{
return geteuid() == 0;
}
#endif /* HAVE_LIBCAP_SUPPORT */
#endif /* __PERF_CAP_H */
[acme@seventh perf]$
>
> > > So I am removing the introduction of perf_cap__capable() from the
> > > first patch you sent, leaving it with _only_ the feature detection
> > > part, using that feature detection to do anything is then moved to a
> > > separate patch, after we finish this discussion about what we should
> > > fallback to when libcap-devel isn't available, i.e. we should use the
> > > previous checks, etc.
> >
> > So, please take a look at the tmp.perf/cap branch in my git repo:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/log/?h=tmp.p
> > erf/cap
> >
> > I split the patch and made perf_cap__capable() fallback to 'return
> > geteuid() == 0;' when libcap-devel isn't available, i.e. keep the checks made
> > prior to your patchset.
>
> Thank you. And thanks for updating "make_minimal".
Ok!
> >
> > Jiri, can I keep your Acked-by?
> >
> > - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev
2019-08-07 14:44 ` [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev
@ 2019-08-07 14:44 ` Igor Lubashev
2019-08-14 18:04 ` Mathieu Poirier
2019-08-07 14:44 ` [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
2019-08-12 9:13 ` [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Jiri Olsa
4 siblings, 1 reply; 33+ messages in thread
From: Igor Lubashev @ 2019-08-07 14:44 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] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-07 14:44 ` [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
@ 2019-08-14 18:04 ` Mathieu Poirier
2019-08-14 18:48 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-14 18:04 UTC (permalink / raw)
To: Igor Lubashev
Cc: Linux Kernel Mailing List, Arnaldo Carvalho de Melo, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
On Wed, 7 Aug 2019 at 08:44, Igor Lubashev <ilubashe@akamai.com> wrote:
>
> 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);
>
# echo 0 > /proc/sys/kernel/kptr_restrict
# ./tools/perf/perf record -e instructions:k uname
perf: Segmentation fault
Obtained 10 stack frames.
./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
/lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
Segmentation fault
I can reproduce this on both x86 and ARM64.
> 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 [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-14 18:04 ` Mathieu Poirier
@ 2019-08-14 18:48 ` Arnaldo Carvalho de Melo
2019-08-14 18:52 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-14 18:48 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Igor Lubashev, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> On Wed, 7 Aug 2019 at 08:44, Igor Lubashev <ilubashe@akamai.com> wrote:
> >
> > 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);
> >
>
> # echo 0 > /proc/sys/kernel/kptr_restrict
> # ./tools/perf/perf record -e instructions:k uname
> perf: Segmentation fault
> Obtained 10 stack frames.
> ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
> ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
> ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
> Segmentation fault
>
> I can reproduce this on both x86 and ARM64.
I don't see this with these two csets removed:
7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
Which were the ones I guessed were related to the problem you reported,
so they are out of my ongoing perf/core pull request to Ingo/Thomas, now
trying with these applied and your instructions...
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-14 18:48 ` Arnaldo Carvalho de Melo
@ 2019-08-14 18:52 ` Arnaldo Carvalho de Melo
2019-08-14 20:02 ` Lubashev, Igor
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-14 18:52 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Igor Lubashev, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > # echo 0 > /proc/sys/kernel/kptr_restrict
> > # ./tools/perf/perf record -e instructions:k uname
> > perf: Segmentation fault
> > Obtained 10 stack frames.
> > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
> > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
> > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
> > Segmentation fault
> >
> > I can reproduce this on both x86 and ARM64.
>
> I don't see this with these two csets removed:
>
> 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
>
> Which were the ones I guessed were related to the problem you reported,
> so they are out of my ongoing perf/core pull request to Ingo/Thomas, now
> trying with these applied and your instructions...
Can't repro:
[root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
0
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]# echo 1 > /proc/sys/kernel/kptr_restrict
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]# echo 0 > /proc/sys/kernel/kptr_restrict
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]#
[acme@quaco perf]$ git log --oneline --author Lubashev tools/
7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap, acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
c766f3df635d perf ftrace: Use CAP_SYS_ADMIN instead of euid==0
c22e150e3afa perf tools: Add helpers to use capabilities if present
74d5f3d06f70 tools build: Add capability-related feature detection
perf version 5.3.rc4.g7ff5b5911144
[acme@quaco perf]$
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-14 18:52 ` Arnaldo Carvalho de Melo
@ 2019-08-14 20:02 ` Lubashev, Igor
2019-08-15 15:01 ` Mathieu Poirier
2019-08-15 20:16 ` Mathieu Poirier
0 siblings, 2 replies; 33+ messages in thread
From: Lubashev, Igor @ 2019-08-14 20:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
> On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > -e instructions:k uname
> > > perf: Segmentation fault
> > > Obtained 10 stack frames.
> > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > [0x55af9e590337]
> > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > [0x7fd31ef99b97]
> > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > >
> > > I can reproduce this on both x86 and ARM64.
> >
> > I don't see this with these two csets removed:
> >
> > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > checks
> >
> > Which were the ones I guessed were related to the problem you
> > reported, so they are out of my ongoing perf/core pull request to
> > Ingo/Thomas, now trying with these applied and your instructions...
>
> Can't repro:
>
> [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> 0
> [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> [root@quaco ~]#
>
> [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> present
> 74d5f3d06f70 tools build: Add capability-related feature detection perf
> version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
I got an ARM64 cloud VM, but I cannot reproduce.
# cat /proc/sys/kernel/kptr_restrict
0
Perf trace works fine (does not die):
# ./perf trace -a
Here is my setup:
Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
Branch: tmp.perf/cap
Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
lsb_release -a: Ubuntu 18.04.3 LTS
Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
... gtk2: [ on ]
... libaudit: [ on ]
... libbfd: [ on ]
... libcap: [ on ]
... libelf: [ on ]
... libnuma: [ on ]
... numa_num_possible_cpus: [ on ]
... libperl: [ on ]
... libpython: [ on ]
... libcrypto: [ on ]
... libunwind: [ on ]
... libdw-dwarf-unwind: [ on ]
... zlib: [ on ]
... lzma: [ on ]
... get_cpuid: [ OFF ]
... bpf: [ on ]
... libaio: [ on ]
... libzstd: [ on ]
... disassembler-four-args: [ on ]
I also could not reproduce on x86:
lsb_release -a: Ubuntu 18.04.1 LTS
gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
uname -r: 4.4.0-154-generic
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-14 20:02 ` Lubashev, Igor
@ 2019-08-15 15:01 ` Mathieu Poirier
2019-08-15 20:16 ` Mathieu Poirier
1 sibling, 0 replies; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-15 15:01 UTC (permalink / raw)
To: Lubashev, Igor
Cc: Arnaldo Carvalho de Melo, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com> wrote:
>
> > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > -e instructions:k uname
> > > > perf: Segmentation fault
> > > > Obtained 10 stack frames.
> > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > [0x55af9e590337]
> > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > [0x7fd31ef99b97]
> > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > >
> > > > I can reproduce this on both x86 and ARM64.
> > >
> > > I don't see this with these two csets removed:
> > >
> > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > checks
> > >
> > > Which were the ones I guessed were related to the problem you
> > > reported, so they are out of my ongoing perf/core pull request to
> > > Ingo/Thomas, now trying with these applied and your instructions...
> >
> > Can't repro:
> >
> > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > [root@quaco ~]#
> >
> > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > present
> > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
>
> I got an ARM64 cloud VM, but I cannot reproduce.
> # cat /proc/sys/kernel/kptr_restrict
> 0
>
> Perf trace works fine (does not die):
> # ./perf trace -a
>
> Here is my setup:
> Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> Branch: tmp.perf/cap
> Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> lsb_release -a: Ubuntu 18.04.3 LTS
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> ... gtk2: [ on ]
> ... libaudit: [ on ]
> ... libbfd: [ on ]
> ... libcap: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ on ]
> ... libdw-dwarf-unwind: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ OFF ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
> ... disassembler-four-args: [ on ]
Thank you for posting your configuration. I will see where things are
different with mine.
>
> I also could not reproduce on x86:
> lsb_release -a: Ubuntu 18.04.1 LTS
> gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> uname -r: 4.4.0-154-generic
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-14 20:02 ` Lubashev, Igor
2019-08-15 15:01 ` Mathieu Poirier
@ 2019-08-15 20:16 ` Mathieu Poirier
2019-08-15 21:42 ` Arnaldo Carvalho de Melo
2019-08-15 22:27 ` Lubashev, Igor
1 sibling, 2 replies; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-15 20:16 UTC (permalink / raw)
To: Lubashev, Igor
Cc: Arnaldo Carvalho de Melo, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
Leo Yan
On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com> wrote:
>
> > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > -e instructions:k uname
> > > > perf: Segmentation fault
> > > > Obtained 10 stack frames.
> > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > [0x55af9e590337]
> > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > [0x7fd31ef99b97]
> > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > >
> > > > I can reproduce this on both x86 and ARM64.
> > >
> > > I don't see this with these two csets removed:
> > >
> > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > checks
> > >
> > > Which were the ones I guessed were related to the problem you
> > > reported, so they are out of my ongoing perf/core pull request to
> > > Ingo/Thomas, now trying with these applied and your instructions...
> >
> > Can't repro:
> >
> > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > [root@quaco ~]#
> >
> > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > present
> > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
>
> I got an ARM64 cloud VM, but I cannot reproduce.
> # cat /proc/sys/kernel/kptr_restrict
> 0
>
> Perf trace works fine (does not die):
> # ./perf trace -a
>
> Here is my setup:
> Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> Branch: tmp.perf/cap
> Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> lsb_release -a: Ubuntu 18.04.3 LTS
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> ... gtk2: [ on ]
> ... libaudit: [ on ]
> ... libbfd: [ on ]
> ... libcap: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ on ]
> ... libdw-dwarf-unwind: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ OFF ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
> ... disassembler-four-args: [ on ]
>
> I also could not reproduce on x86:
> lsb_release -a: Ubuntu 18.04.1 LTS
> gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> uname -r: 4.4.0-154-generic
I isolated the problem to libcap-dev - if it is not installed a
segmentation fault will occur. Since this set is about using
capabilities it is obvious that not having it on a system should fail
a trace session, but it should not crash it.
If libcap-dev is not installed function symbol__restricted_filename()
will return true, which in turn will prevent symbol_name to be set in
machine__get_running_kernel_start(). That prevents function
map__set_kallsyms_ref_reloc_sym() from being called in
machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
fault.
I am not sure how this can be fixed. I counted a total of 19
instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
care to check if kmap->ref_reloc_sym is valid before proceeding. As
such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
guaranteed to be valid. If I am correct then all we need is to check
for a valid pointer in _perf_event__synthesize_kernel_mmap().
Otherwise it will be a little harder.
Mathieu
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-15 20:16 ` Mathieu Poirier
@ 2019-08-15 21:42 ` Arnaldo Carvalho de Melo
2019-08-19 16:51 ` Mathieu Poirier
2019-08-15 22:27 ` Lubashev, Igor
1 sibling, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-15 21:42 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Lubashev, Igor, Arnaldo Carvalho de Melo,
Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris, Leo Yan
Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com> wrote:
> >
> > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > escreveu:
> > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > > -e instructions:k uname
> > > > > perf: Segmentation fault
> > > > > Obtained 10 stack frames.
> > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > [0x55af9e590337]
> > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > [0x7fd31ef99b97]
> > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > > >
> > > > > I can reproduce this on both x86 and ARM64.
> > > >
> > > > I don't see this with these two csets removed:
> > > >
> > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > > checks
> > > >
> > > > Which were the ones I guessed were related to the problem you
> > > > reported, so they are out of my ongoing perf/core pull request to
> > > > Ingo/Thomas, now trying with these applied and your instructions...
> > >
> > > Can't repro:
> > >
> > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > 0
> > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > > [root@quaco ~]#
> > >
> > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > > present
> > > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> >
> > I got an ARM64 cloud VM, but I cannot reproduce.
> > # cat /proc/sys/kernel/kptr_restrict
> > 0
> >
> > Perf trace works fine (does not die):
> > # ./perf trace -a
> >
> > Here is my setup:
> > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > Branch: tmp.perf/cap
> > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> > lsb_release -a: Ubuntu 18.04.3 LTS
> >
> > Auto-detecting system features:
> > ... dwarf: [ on ]
> > ... dwarf_getlocations: [ on ]
> > ... glibc: [ on ]
> > ... gtk2: [ on ]
> > ... libaudit: [ on ]
> > ... libbfd: [ on ]
> > ... libcap: [ on ]
> > ... libelf: [ on ]
> > ... libnuma: [ on ]
> > ... numa_num_possible_cpus: [ on ]
> > ... libperl: [ on ]
> > ... libpython: [ on ]
> > ... libcrypto: [ on ]
> > ... libunwind: [ on ]
> > ... libdw-dwarf-unwind: [ on ]
> > ... zlib: [ on ]
> > ... lzma: [ on ]
> > ... get_cpuid: [ OFF ]
> > ... bpf: [ on ]
> > ... libaio: [ on ]
> > ... libzstd: [ on ]
> > ... disassembler-four-args: [ on ]
> >
> > I also could not reproduce on x86:
> > lsb_release -a: Ubuntu 18.04.1 LTS
> > gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> > uname -r: 4.4.0-154-generic
>
> I isolated the problem to libcap-dev - if it is not installed a
> segmentation fault will occur. Since this set is about using
> capabilities it is obvious that not having it on a system should fail
> a trace session, but it should not crash it.
It shouldn't crash on x86_64:
root@quaco ~]# sysctl kernel.kptr_restrict
kernel.kptr_restrict = 0
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.023 MB perf.data (5 samples) ]
[root@quaco ~]# ldd ~/bin/perf | grep libcap
[root@quaco ~]# perf -v
perf version 5.3.rc4.g329ca330bf8b
[root@quaco ~]#
[acme@quaco perf]$ git log --oneline -4
329ca330bf8b (HEAD -> perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
f7b9999387af perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf report: Add --switch-on/--switch-off events
2f53ae347f59 perf top: Add --switch-on/--switch-off events
[acme@quaco perf]$
> If libcap-dev is not installed function symbol__restricted_filename()
> will return true, which in turn will prevent symbol_name to be set in
> machine__get_running_kernel_start(). That prevents function
> map__set_kallsyms_ref_reloc_sym() from being called in
> machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
> fault.
Can you please try with my perf/cap branch? Perhaps you're using Igor's
original set of patches? I made changes to the fallback, he was making
it return false while I made it fallback to geteuid() == 0, as was
before his patches.
- Arnaldo
> I am not sure how this can be fixed. I counted a total of 19
> instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> care to check if kmap->ref_reloc_sym is valid before proceeding. As
> such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> guaranteed to be valid. If I am correct then all we need is to check
> for a valid pointer in _perf_event__synthesize_kernel_mmap().
> Otherwise it will be a little harder.
>
> Mathieu
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-15 21:42 ` Arnaldo Carvalho de Melo
@ 2019-08-19 16:51 ` Mathieu Poirier
2019-08-19 22:22 ` Lubashev, Igor
0 siblings, 1 reply; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-19 16:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Lubashev, Igor, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
Leo Yan
On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com> wrote:
> > >
> > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > > escreveu:
> > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > > > -e instructions:k uname
> > > > > > perf: Segmentation fault
> > > > > > Obtained 10 stack frames.
> > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > [0x55af9e590337]
> > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > [0x7fd31ef99b97]
> > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > > > >
> > > > > > I can reproduce this on both x86 and ARM64.
> > > > >
> > > > > I don't see this with these two csets removed:
> > > > >
> > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > > > checks
> > > > >
> > > > > Which were the ones I guessed were related to the problem you
> > > > > reported, so they are out of my ongoing perf/core pull request to
> > > > > Ingo/Thomas, now trying with these applied and your instructions...
> > > >
> > > > Can't repro:
> > > >
> > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > 0
> > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > > > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > > > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > > > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > > > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > > > [root@quaco ~]#
> > > >
> > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > > > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > > > present
> > > > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > > > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > >
> > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > # cat /proc/sys/kernel/kptr_restrict
> > > 0
> > >
> > > Perf trace works fine (does not die):
> > > # ./perf trace -a
> > >
> > > Here is my setup:
> > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > Branch: tmp.perf/cap
> > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> > > lsb_release -a: Ubuntu 18.04.3 LTS
> > >
> > > Auto-detecting system features:
> > > ... dwarf: [ on ]
> > > ... dwarf_getlocations: [ on ]
> > > ... glibc: [ on ]
> > > ... gtk2: [ on ]
> > > ... libaudit: [ on ]
> > > ... libbfd: [ on ]
> > > ... libcap: [ on ]
> > > ... libelf: [ on ]
> > > ... libnuma: [ on ]
> > > ... numa_num_possible_cpus: [ on ]
> > > ... libperl: [ on ]
> > > ... libpython: [ on ]
> > > ... libcrypto: [ on ]
> > > ... libunwind: [ on ]
> > > ... libdw-dwarf-unwind: [ on ]
> > > ... zlib: [ on ]
> > > ... lzma: [ on ]
> > > ... get_cpuid: [ OFF ]
> > > ... bpf: [ on ]
> > > ... libaio: [ on ]
> > > ... libzstd: [ on ]
> > > ... disassembler-four-args: [ on ]
> > >
> > > I also could not reproduce on x86:
> > > lsb_release -a: Ubuntu 18.04.1 LTS
> > > gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> > > uname -r: 4.4.0-154-generic
> >
> > I isolated the problem to libcap-dev - if it is not installed a
> > segmentation fault will occur. Since this set is about using
> > capabilities it is obvious that not having it on a system should fail
> > a trace session, but it should not crash it.
>
> It shouldn't crash on x86_64:
>
> root@quaco ~]# sysctl kernel.kptr_restrict
> kernel.kptr_restrict = 0
> [root@quaco ~]# perf record -e instructions:k uname
> Linux
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.023 MB perf.data (5 samples) ]
> [root@quaco ~]# ldd ~/bin/perf | grep libcap
> [root@quaco ~]# perf -v
> perf version 5.3.rc4.g329ca330bf8b
> [root@quaco ~]#
> [acme@quaco perf]$ git log --oneline -4
> 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> f7b9999387af perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf report: Add --switch-on/--switch-off events
> 2f53ae347f59 perf top: Add --switch-on/--switch-off events
> [acme@quaco perf]$
>
> > If libcap-dev is not installed function symbol__restricted_filename()
> > will return true, which in turn will prevent symbol_name to be set in
> > machine__get_running_kernel_start(). That prevents function
> > map__set_kallsyms_ref_reloc_sym() from being called in
> > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> > NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
> > fault.
>
> Can you please try with my perf/cap branch? Perhaps you're using Igor's
> original set of patches? I made changes to the fallback, he was making
> it return false while I made it fallback to geteuid() == 0, as was
> before his patches.
Things are working properly on your perf/cap branch. I tested with on
both x86 and ARM.
Mathieu
>
> - Arnaldo
>
> > I am not sure how this can be fixed. I counted a total of 19
> > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > guaranteed to be valid. If I am correct then all we need is to check
> > for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > Otherwise it will be a little harder.
> >
> > Mathieu
>
> --
>
> - Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-19 16:51 ` Mathieu Poirier
@ 2019-08-19 22:22 ` Lubashev, Igor
2019-08-20 16:57 ` Mathieu Poirier
2019-08-20 17:13 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 33+ messages in thread
From: Lubashev, Igor @ 2019-08-19 22:22 UTC (permalink / raw)
To: Mathieu Poirier, Arnaldo Carvalho de Melo
Cc: Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris, Leo Yan
On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com>
> wrote:
> > > >
> > > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de
> > > > > Melo
> > > > > escreveu:
> > > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier
> escreveu:
> > > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict #
> > > > > > > ./tools/perf/perf record -e instructions:k uname
> > > > > > > perf: Segmentation fault
> > > > > > > Obtained 10 stack frames.
> > > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44)
> > > > > > > [0x55af9e5da5d4]
> > > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > > [0x55af9e590337]
> > > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > > [0x7fd31ef99b97]
> > > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > > > fault
> > > > > > >
> > > > > > > I can reproduce this on both x86 and ARM64.
> > > > > >
> > > > > > I don't see this with these two csets removed:
> > > > > >
> > > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > > > perf_event_paranoid checks
> > > > > >
> > > > > > Which were the ones I guessed were related to the problem you
> > > > > > reported, so they are out of my ongoing perf/core pull request
> > > > > > to Ingo/Thomas, now trying with these applied and your
> instructions...
> > > > >
> > > > > Can't repro:
> > > > >
> > > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > > 0
> > > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> record:
> > > > > Woken up 1 times to write data ] [ perf record: Captured and
> > > > > wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > > instructions:k uname Linux [ perf record: Woken up 1 times to write
> data ] [ perf record:
> > > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco
> > > > > ~]# echo
> > > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record
> > > > > -e instructions:k uname Linux [ perf record: Woken up 1 times to
> > > > > write data ] [ perf record: Captured and wrote 0.024 MB
> > > > > perf.data (1 samples) ] [root@quaco ~]#
> > > > >
> > > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with
> > > > > kptr_restrict checks d7604b66102e perf tools: Use CAP_SYS_ADMIN
> > > > > with perf_event_paranoid checks c766f3df635d perf ftrace: Use
> > > > > CAP_SYS_ADMIN instead of euid==0 c22e150e3afa perf tools: Add
> > > > > helpers to use capabilities if present
> > > > > 74d5f3d06f70 tools build: Add capability-related feature
> > > > > detection perf version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > > >
> > > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > > # cat /proc/sys/kernel/kptr_restrict
> > > > 0
> > > >
> > > > Perf trace works fine (does not die):
> > > > # ./perf trace -a
> > > >
> > > > Here is my setup:
> > > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > > Branch: tmp.perf/cap
> > > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict
> checks"
> > > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10
> > > > 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux lsb_release
> > > > -a: Ubuntu 18.04.3 LTS
> > > >
> > > > Auto-detecting system features:
> > > > ... dwarf: [ on ]
> > > > ... dwarf_getlocations: [ on ]
> > > > ... glibc: [ on ]
> > > > ... gtk2: [ on ]
> > > > ... libaudit: [ on ]
> > > > ... libbfd: [ on ]
> > > > ... libcap: [ on ]
> > > > ... libelf: [ on ]
> > > > ... libnuma: [ on ]
> > > > ... numa_num_possible_cpus: [ on ]
> > > > ... libperl: [ on ]
> > > > ... libpython: [ on ]
> > > > ... libcrypto: [ on ]
> > > > ... libunwind: [ on ]
> > > > ... libdw-dwarf-unwind: [ on ]
> > > > ... zlib: [ on ]
> > > > ... lzma: [ on ]
> > > > ... get_cpuid: [ OFF ]
> > > > ... bpf: [ on ]
> > > > ... libaio: [ on ]
> > > > ... libzstd: [ on ]
> > > > ... disassembler-four-args: [ on ]
> > > >
> > > > I also could not reproduce on x86:
> > > > lsb_release -a: Ubuntu 18.04.1 LTS gcc --version: gcc (Ubuntu
> > > > 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0 uname -r: 4.4.0-154-generic
> > >
> > > I isolated the problem to libcap-dev - if it is not installed a
> > > segmentation fault will occur. Since this set is about using
> > > capabilities it is obvious that not having it on a system should
> > > fail a trace session, but it should not crash it.
> >
> > It shouldn't crash on x86_64:
> >
> > root@quaco ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > record: Woken up 1 times to write data ] [ perf record: Captured and
> > wrote 0.023 MB perf.data (5 samples) ] [root@quaco ~]# ldd ~/bin/perf
> > | grep libcap [root@quaco ~]# perf -v perf version
> > 5.3.rc4.g329ca330bf8b [root@quaco ~]# [acme@quaco perf]$ git log
> > --oneline -4 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use
> > CAP_SYSLOG with kptr_restrict checks f7b9999387af perf tools: Use
> > CAP_SYS_ADMIN with perf_event_paranoid checks
> > 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf
> > report: Add --switch-on/--switch-off events
> > 2f53ae347f59 perf top: Add --switch-on/--switch-off events [acme@quaco
> > perf]$
> >
> > > If libcap-dev is not installed function
> > > symbol__restricted_filename() will return true, which in turn will
> > > prevent symbol_name to be set in
> > > machine__get_running_kernel_start(). That prevents function
> > > map__set_kallsyms_ref_reloc_sym() from being called in
> > > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym
> > > being NULL in _perf_event__synthesize_kernel_mmap() and a
> > > segmentation fault.
> >
> > Can you please try with my perf/cap branch? Perhaps you're using
> > Igor's original set of patches? I made changes to the fallback, he was
> > making it return false while I made it fallback to geteuid() == 0, as
> > was before his patches.
>
> Things are working properly on your perf/cap branch. I tested with on both
> x86 and ARM.
Mathieu, you are probably testing with euid==0. If you were to test with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0, perf_event_paranoid=2), you would likely hit the bug that you identified in __perf_event__synthesize_kermel_mmap().
See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).
Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.
- Igor
> > > I am not sure how this can be fixed. I counted a total of 19
> > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > guaranteed to be valid. If I am correct then all we need is to
> > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > Otherwise it will be a little harder.
> > >
> > > Mathieu
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-19 22:22 ` Lubashev, Igor
@ 2019-08-20 16:57 ` Mathieu Poirier
2019-08-20 17:13 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-20 16:57 UTC (permalink / raw)
To: Lubashev, Igor
Cc: Arnaldo Carvalho de Melo, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
Leo Yan
On Mon, 19 Aug 2019 at 16:22, Lubashev, Igor <ilubashe@akamai.com> wrote:
>
> On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> > On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > > > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com>
> > wrote:
> > > > >
> > > > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > > > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de
> > > > > > Melo
> > > > > > escreveu:
> > > > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier
> > escreveu:
> > > > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict #
> > > > > > > > ./tools/perf/perf record -e instructions:k uname
> > > > > > > > perf: Segmentation fault
> > > > > > > > Obtained 10 stack frames.
> > > > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44)
> > > > > > > > [0x55af9e5da5d4]
> > > > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > > > [0x55af9e590337]
> > > > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > > > [0x7fd31ef99b97]
> > > > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > > > > fault
> > > > > > > >
> > > > > > > > I can reproduce this on both x86 and ARM64.
> > > > > > >
> > > > > > > I don't see this with these two csets removed:
> > > > > > >
> > > > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > > > > perf_event_paranoid checks
> > > > > > >
> > > > > > > Which were the ones I guessed were related to the problem you
> > > > > > > reported, so they are out of my ongoing perf/core pull request
> > > > > > > to Ingo/Thomas, now trying with these applied and your
> > instructions...
> > > > > >
> > > > > > Can't repro:
> > > > > >
> > > > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > > > 0
> > > > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > record:
> > > > > > Woken up 1 times to write data ] [ perf record: Captured and
> > > > > > wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > > > instructions:k uname Linux [ perf record: Woken up 1 times to write
> > data ] [ perf record:
> > > > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco
> > > > > > ~]# echo
> > > > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record
> > > > > > -e instructions:k uname Linux [ perf record: Woken up 1 times to
> > > > > > write data ] [ perf record: Captured and wrote 0.024 MB
> > > > > > perf.data (1 samples) ] [root@quaco ~]#
> > > > > >
> > > > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with
> > > > > > kptr_restrict checks d7604b66102e perf tools: Use CAP_SYS_ADMIN
> > > > > > with perf_event_paranoid checks c766f3df635d perf ftrace: Use
> > > > > > CAP_SYS_ADMIN instead of euid==0 c22e150e3afa perf tools: Add
> > > > > > helpers to use capabilities if present
> > > > > > 74d5f3d06f70 tools build: Add capability-related feature
> > > > > > detection perf version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > > > >
> > > > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > > > # cat /proc/sys/kernel/kptr_restrict
> > > > > 0
> > > > >
> > > > > Perf trace works fine (does not die):
> > > > > # ./perf trace -a
> > > > >
> > > > > Here is my setup:
> > > > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > > > Branch: tmp.perf/cap
> > > > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks"
> > > > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10
> > > > > 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux lsb_release
> > > > > -a: Ubuntu 18.04.3 LTS
> > > > >
> > > > > Auto-detecting system features:
> > > > > ... dwarf: [ on ]
> > > > > ... dwarf_getlocations: [ on ]
> > > > > ... glibc: [ on ]
> > > > > ... gtk2: [ on ]
> > > > > ... libaudit: [ on ]
> > > > > ... libbfd: [ on ]
> > > > > ... libcap: [ on ]
> > > > > ... libelf: [ on ]
> > > > > ... libnuma: [ on ]
> > > > > ... numa_num_possible_cpus: [ on ]
> > > > > ... libperl: [ on ]
> > > > > ... libpython: [ on ]
> > > > > ... libcrypto: [ on ]
> > > > > ... libunwind: [ on ]
> > > > > ... libdw-dwarf-unwind: [ on ]
> > > > > ... zlib: [ on ]
> > > > > ... lzma: [ on ]
> > > > > ... get_cpuid: [ OFF ]
> > > > > ... bpf: [ on ]
> > > > > ... libaio: [ on ]
> > > > > ... libzstd: [ on ]
> > > > > ... disassembler-four-args: [ on ]
> > > > >
> > > > > I also could not reproduce on x86:
> > > > > lsb_release -a: Ubuntu 18.04.1 LTS gcc --version: gcc (Ubuntu
> > > > > 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0 uname -r: 4.4.0-154-generic
> > > >
> > > > I isolated the problem to libcap-dev - if it is not installed a
> > > > segmentation fault will occur. Since this set is about using
> > > > capabilities it is obvious that not having it on a system should
> > > > fail a trace session, but it should not crash it.
> > >
> > > It shouldn't crash on x86_64:
> > >
> > > root@quaco ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0
> > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > > record: Woken up 1 times to write data ] [ perf record: Captured and
> > > wrote 0.023 MB perf.data (5 samples) ] [root@quaco ~]# ldd ~/bin/perf
> > > | grep libcap [root@quaco ~]# perf -v perf version
> > > 5.3.rc4.g329ca330bf8b [root@quaco ~]# [acme@quaco perf]$ git log
> > > --oneline -4 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use
> > > CAP_SYSLOG with kptr_restrict checks f7b9999387af perf tools: Use
> > > CAP_SYS_ADMIN with perf_event_paranoid checks
> > > 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf
> > > report: Add --switch-on/--switch-off events
> > > 2f53ae347f59 perf top: Add --switch-on/--switch-off events [acme@quaco
> > > perf]$
> > >
> > > > If libcap-dev is not installed function
> > > > symbol__restricted_filename() will return true, which in turn will
> > > > prevent symbol_name to be set in
> > > > machine__get_running_kernel_start(). That prevents function
> > > > map__set_kallsyms_ref_reloc_sym() from being called in
> > > > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym
> > > > being NULL in _perf_event__synthesize_kernel_mmap() and a
> > > > segmentation fault.
> > >
> > > Can you please try with my perf/cap branch? Perhaps you're using
> > > Igor's original set of patches? I made changes to the fallback, he was
> > > making it return false while I made it fallback to geteuid() == 0, as
> > > was before his patches.
> >
> > Things are working properly on your perf/cap branch. I tested with on both
> > x86 and ARM.
>
> Mathieu, you are probably testing with euid==0.
Very true
> If you were to test with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0, perf_event_paranoid=2), you would likely hit the bug that you identified in __perf_event__synthesize_kermel_mmap().
>
> See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).
>
> Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.
CC me on the next patchset and I'll be happy to test it.
>
> - Igor
>
>
> > > > I am not sure how this can be fixed. I counted a total of 19
> > > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > > guaranteed to be valid. If I am correct then all we need is to
> > > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > > Otherwise it will be a little harder.
> > > >
> > > > Mathieu
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-19 22:22 ` Lubashev, Igor
2019-08-20 16:57 ` Mathieu Poirier
@ 2019-08-20 17:13 ` Arnaldo Carvalho de Melo
2019-08-27 1:58 ` Lubashev, Igor
1 sibling, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-20 17:13 UTC (permalink / raw)
To: Lubashev, Igor
Cc: Mathieu Poirier, Arnaldo Carvalho de Melo,
Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris, Leo Yan
Em Mon, Aug 19, 2019 at 10:22:07PM +0000, Lubashev, Igor escreveu:
> On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> > On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Things are working properly on your perf/cap branch. I tested with on both
> > x86 and ARM.
> Mathieu, you are probably testing with euid==0. If you were to test
> with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0,
> perf_event_paranoid=2), you would likely hit the bug that you
> identified in __perf_event__synthesize_kermel_mmap().
> See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).
>
> Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.
I think you should get the checks for ref_reloc_sym in place so as to
make the code overall more robust, and also go on continuing to make the
checks in tools/perf/ to match what is checked on the other side of the
mirror, i.e. by the kernel, so from a quick read, please put first the
robustness patches (check ref_reloc_sym) do your other suggestions and
update the warnings, then refresh the two patches that still are not in
my perf/core branch:
[acme@quaco perf]$ git rebase perf/core
First, rewinding head to replay your work on top of it...
Applying: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
Applying: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
[acme@quaco perf]$
I've pushed out perf/cap, so you can go from there as it is rebased on
my current perf/core.
Then test all these cases: with/without libcap, with euid==0 and
different than zero, with capabilities, etc, patch by patch so that we
don't break bisection nor regress,
Thanks and keep up the good work!
- Arnaldo
> - Igor
>
>
> > > > I am not sure how this can be fixed. I counted a total of 19
> > > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > > guaranteed to be valid. If I am correct then all we need is to
> > > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > > Otherwise it will be a little harder.
> > > >
> > > > Mathieu
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-20 17:13 ` Arnaldo Carvalho de Melo
@ 2019-08-27 1:58 ` Lubashev, Igor
0 siblings, 0 replies; 33+ messages in thread
From: Lubashev, Igor @ 2019-08-27 1:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Mathieu Poirier
Cc: Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, Leo Yan
On Tue, August 20, 2019 at 1:14 PM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > Arnaldo, once we decide what the right fix is, I am happy to post the
> update (options 1, 1+2) as a patch series.
>
> I think you should get the checks for ref_reloc_sym in place so as to make the
> code overall more robust, and also go on continuing to make the checks in
> tools/perf/ to match what is checked on the other side of the mirror, i.e. by
> the kernel, so from a quick read, please put first the robustness patches
> (check ref_reloc_sym) do your other suggestions and update the warnings,
> then refresh the two patches that still are not in my perf/core branch:
>
> [acme@quaco perf]$ git rebase perf/core
> First, rewinding head to replay your work on top of it...
> Applying: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> Applying: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> [acme@quaco perf]$
>
> I've pushed out perf/cap, so you can go from there as it is rebased on my
> current perf/core.
>
> Then test all these cases: with/without libcap, with euid==0 and different
> than zero, with capabilities, etc, patch by patch so that we don't break
> bisection nor regress,
All done. I've posted the update as a new follow-up series: https://lkml.kernel.org/lkml/1566869956-7154-1-git-send-email-ilubashe@akamai.com/ rebased on your perf/core.
I've tested 336 permutations (see the new cover). In particular, I was able to reproduce the crash on perf/cap and confirm that no permutation can cause such crashes for any of the patches in the series.
> Thanks and keep up the good work!
>
> - Arnaldo
- Igor
^ permalink raw reply [flat|nested] 33+ messages in thread
* RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks
2019-08-15 20:16 ` Mathieu Poirier
2019-08-15 21:42 ` Arnaldo Carvalho de Melo
@ 2019-08-15 22:27 ` Lubashev, Igor
1 sibling, 0 replies; 33+ messages in thread
From: Lubashev, Igor @ 2019-08-15 22:27 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Arnaldo Carvalho de Melo, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris,
Leo Yan
On Thu, August 15, 2019 at 4:17 PM Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
> On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <ilubashe@akamai.com>
> wrote:
> >
> > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > escreveu:
> > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf
> > > > > record -e instructions:k uname
> > > > > perf: Segmentation fault
> > > > > Obtained 10 stack frames.
> > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > [0x55af9e590337]
> > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > [0x7fd31ef99b97]
> > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > fault
> > > > >
> > > > > I can reproduce this on both x86 and ARM64.
> > > >
> > > > I don't see this with these two csets removed:
> > > >
> > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > perf_event_paranoid checks
> > > >
> > > > Which were the ones I guessed were related to the problem you
> > > > reported, so they are out of my ongoing perf/core pull request to
> > > > Ingo/Thomas, now trying with these applied and your instructions...
SNIP
> I isolated the problem to libcap-dev - if it is not installed a segmentation fault
> will occur. Since this set is about using capabilities it is obvious that not
> having it on a system should fail a trace session, but it should not crash it.
>
> If libcap-dev is not installed function symbol__restricted_filename() will
> return true, which in turn will prevent symbol_name to be set in
> machine__get_running_kernel_start(). That prevents function
> map__set_kallsyms_ref_reloc_sym() from being called in
> machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> NULL in _perf_event__synthesize_kernel_mmap() and a segmentation fault.
Thank you, great find.
> I am not sure how this can be fixed. I counted a total of 19 instances where
> kmap->ref_reloc_sym->XYZ is called, only 2 of wich care to check if kmap-
> >ref_reloc_sym is valid before proceeding. As such I must hope that in the
> 17 other cases, kmap->ref_reloc_sym is guaranteed to be valid. If I am
> correct then all we need is to check for a valid pointer in
> _perf_event__synthesize_kernel_mmap().
> Otherwise it will be a little harder.
I also see 19 instances in 5 files, but by my inspection all cases but one are ok (the code checks for NULL earlier in the function or in a helper function).
The not ok case is __perf_event__synthesize_kermel_mmap(), which simply checks symbol_conf.kptr_restrict.
==== Option 1 =====
Fix __perf_event__synthesize_kermel_mmap(). This probably should be done regardless.
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));
--
==== Option 2 =====
Move the new perf_event_paranoid() check from symbol__restricted_filename() to symbol__read_kptr_restrict().
Other than the use above, kptr_restrict is only used for printing warnings.
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7409d2facd5b..035f2e75728c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -898,11 +898,7 @@ bool symbol__restricted_filename(const char *filename,
{
bool restricted = false;
- /* 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))) {
+ if (symbol_conf.kptr_restrict) {
char *r = realpath(filename, NULL);
if (r != NULL) {
@@ -2209,6 +2205,12 @@ static bool symbol__read_kptr_restrict(void)
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;
}
--------- And then update the warnings. -----------
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;
- Igor
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
` (2 preceding siblings ...)
2019-08-07 14:44 ` [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev
@ 2019-08-07 14:44 ` Igor Lubashev
2019-08-12 20:22 ` Arnaldo Carvalho de Melo
2019-08-15 9:27 ` [tip:perf/core] perf ftrace: Use CAP_SYS_ADMIN instead of euid==0 tip-bot for Igor Lubashev
2019-08-12 9:13 ` [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Jiri Olsa
4 siblings, 2 replies; 33+ messages in thread
From: Igor Lubashev @ 2019-08-07 14:44 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] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-07 14:44 ` [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
@ 2019-08-12 20:22 ` Arnaldo Carvalho de Melo
2019-08-12 20:27 ` Arnaldo Carvalho de Melo
2019-08-15 9:27 ` [tip:perf/core] perf ftrace: Use CAP_SYS_ADMIN instead of euid==0 tip-bot for Igor Lubashev
1 sibling, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 20:22 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> 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");
I guess we should update the error message too?
> return -1;
> }
> --
> 2.7.4
--
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-12 20:22 ` Arnaldo Carvalho de Melo
@ 2019-08-12 20:27 ` Arnaldo Carvalho de Melo
2019-08-12 20:29 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 20:27 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > 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");
>
> I guess we should update the error message too?
>
I.e. I applied this as a follow up patch:
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 01a5bb58eb04..ba8b65c2f9dc 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
};
if (!perf_cap__capable(CAP_SYS_ADMIN)) {
- pr_err("ftrace only works for root!\n");
+ pr_err("ftrace only works for %s!\n",
+#ifdef HAVE_LIBCAP_SUPPORT
+ "users with the SYS_ADMIN capability"
+#else
+ "root"
+#endif
return -1;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-12 20:27 ` Arnaldo Carvalho de Melo
@ 2019-08-12 20:29 ` Arnaldo Carvalho de Melo
2019-08-12 21:42 ` Mathieu Poirier
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-12 20:29 UTC (permalink / raw)
To: Igor Lubashev
Cc: linux-kernel, Jiri Olsa, Alexey Budankov, Peter Zijlstra,
Ingo Molnar, Mathieu Poirier, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Mon, Aug 12, 2019 at 05:27:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > > @@ -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");
> >
> > I guess we should update the error message too?
> >
>
> I.e. I applied this as a follow up patch:
>
> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> index 01a5bb58eb04..ba8b65c2f9dc 100644
> --- a/tools/perf/builtin-ftrace.c
> +++ b/tools/perf/builtin-ftrace.c
> @@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> };
>
> if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> - pr_err("ftrace only works for root!\n");
> + pr_err("ftrace only works for %s!\n",
> +#ifdef HAVE_LIBCAP_SUPPORT
> + "users with the SYS_ADMIN capability"
> +#else
> + "root"
> +#endif
);
:-)
> return -1;
> }
>
I've pushed the whole set to my tmp.perf/cap branch, please chec
- Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-12 20:29 ` Arnaldo Carvalho de Melo
@ 2019-08-12 21:42 ` Mathieu Poirier
2019-08-13 13:23 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-12 21:42 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Igor Lubashev, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
On Mon, 12 Aug 2019 at 14:29, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Mon, Aug 12, 2019 at 05:27:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > > > @@ -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");
> > >
> > > I guess we should update the error message too?
> > >
> >
> > I.e. I applied this as a follow up patch:
> >
> > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > index 01a5bb58eb04..ba8b65c2f9dc 100644
> > --- a/tools/perf/builtin-ftrace.c
> > +++ b/tools/perf/builtin-ftrace.c
> > @@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > };
> >
> > if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> > - pr_err("ftrace only works for root!\n");
> > + pr_err("ftrace only works for %s!\n",
> > +#ifdef HAVE_LIBCAP_SUPPORT
> > + "users with the SYS_ADMIN capability"
> > +#else
> > + "root"
> > +#endif
>
> );
>
> :-)
>
> > return -1;
> > }
> >
>
> I've pushed the whole set to my tmp.perf/cap branch, please chec
Please hold on before moving further - I'm getting a segmentation
fault on ARM64 that I'm still trying to figure out.
>
> - Arnaldo
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-12 21:42 ` Mathieu Poirier
@ 2019-08-13 13:23 ` Arnaldo Carvalho de Melo
2019-08-13 16:35 ` Mathieu Poirier
0 siblings, 1 reply; 33+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-13 13:23 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Arnaldo Carvalho de Melo, Igor Lubashev,
Linux Kernel Mailing List, Jiri Olsa, Alexey Budankov,
Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
Suzuki K Poulose, linux-arm-kernel, James Morris
Em Mon, Aug 12, 2019 at 03:42:17PM -0600, Mathieu Poirier escreveu:
> On Mon, 12 Aug 2019 at 14:29, Arnaldo Carvalho de Melo
> <arnaldo.melo@gmail.com> wrote:
> >
> > Em Mon, Aug 12, 2019 at 05:27:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > > > > @@ -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");
> > > >
> > > > I guess we should update the error message too?
> > > >
> > >
> > > I.e. I applied this as a follow up patch:
> > >
> > > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > > index 01a5bb58eb04..ba8b65c2f9dc 100644
> > > --- a/tools/perf/builtin-ftrace.c
> > > +++ b/tools/perf/builtin-ftrace.c
> > > @@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > > };
> > >
> > > if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> > > - pr_err("ftrace only works for root!\n");
> > > + pr_err("ftrace only works for %s!\n",
> > > +#ifdef HAVE_LIBCAP_SUPPORT
> > > + "users with the SYS_ADMIN capability"
> > > +#else
> > > + "root"
> > > +#endif
> >
> > );
> >
> > :-)
> >
> > > return -1;
> > > }
> > >
> >
> > I've pushed the whole set to my tmp.perf/cap branch, please chec
>
> Please hold on before moving further - I'm getting a segmentation
> fault on ARM64 that I'm still trying to figure out.
This is just sitting in my tmp branch, and in my local perf/core branch,
so that I can test it with the containers, etc.
Is this related to the following fix?
commit 3e70008a6021fffd2cd1614734603ea970773060
Author: Leo Yan <leo.yan@linaro.org>
Date: Fri Aug 9 18:47:52 2019 +0800
perf trace: Fix segmentation fault when access syscall info on arm64
'perf trace' reports the segmentation fault as below on Arm64:
# perf trace -e string -e augmented_raw_syscalls.c
LLVM: dumping tools/perf/examples/bpf/augmented_raw_syscalls.o
perf: Segmentation fault
Obtained 12 stack frames.
perf(sighandler_dump_stack+0x47) [0xaaaaac96ac87]
linux-vdso.so.1(+0x5b7) [0xffffadbeb5b7]
/lib/aarch64-linux-gnu/libc.so.6(strlen+0x10) [0xfffface7d5d0]
/lib/aarch64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ac7) [0xfffface49f97]
/lib/aarch64-linux-gnu/libc.so.6(__vsnprintf_chk+0xc7) [0xffffacedfbe7]
perf(scnprintf+0x97) [0xaaaaac9ca3ff]
perf(+0x997bb) [0xaaaaac8e37bb]
perf(cmd_trace+0x28e7) [0xaaaaac8ec09f]
perf(+0xd4a13) [0xaaaaac91ea13]
perf(main+0x62f) [0xaaaaac8a147f]
/lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe3) [0xfffface22d23]
perf(+0x57723) [0xaaaaac8a1723]
Segmentation fault
This issue is introduced by commit 30a910d7d3e0 ("perf trace:
Preallocate the syscall table"), it allocates trace->syscalls.table[]
array and the element count is 'trace->sctbl->syscalls.nr_entries'; but
on Arm64, the system call number is not continuously used; e.g. the
syscall maximum id is 436 but the real entries is only 281.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
2019-08-13 13:23 ` Arnaldo Carvalho de Melo
@ 2019-08-13 16:35 ` Mathieu Poirier
0 siblings, 0 replies; 33+ messages in thread
From: Mathieu Poirier @ 2019-08-13 16:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Igor Lubashev, Linux Kernel Mailing List, Jiri Olsa,
Alexey Budankov, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
Namhyung Kim, Suzuki K Poulose, linux-arm-kernel, James Morris
Hi Arnaldo,
On Tue, 13 Aug 2019 at 07:23, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Mon, Aug 12, 2019 at 03:42:17PM -0600, Mathieu Poirier escreveu:
> > On Mon, 12 Aug 2019 at 14:29, Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Mon, Aug 12, 2019 at 05:27:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Mon, Aug 12, 2019 at 05:22:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > Em Wed, Aug 07, 2019 at 10:44:17AM -0400, Igor Lubashev escreveu:
> > > > > > @@ -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");
> > > > >
> > > > > I guess we should update the error message too?
> > > > >
> > > >
> > > > I.e. I applied this as a follow up patch:
> > > >
> > > > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
> > > > index 01a5bb58eb04..ba8b65c2f9dc 100644
> > > > --- a/tools/perf/builtin-ftrace.c
> > > > +++ b/tools/perf/builtin-ftrace.c
> > > > @@ -284,7 +284,12 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv)
> > > > };
> > > >
> > > > if (!perf_cap__capable(CAP_SYS_ADMIN)) {
> > > > - pr_err("ftrace only works for root!\n");
> > > > + pr_err("ftrace only works for %s!\n",
> > > > +#ifdef HAVE_LIBCAP_SUPPORT
> > > > + "users with the SYS_ADMIN capability"
> > > > +#else
> > > > + "root"
> > > > +#endif
> > >
> > > );
> > >
> > > :-)
> > >
> > > > return -1;
> > > > }
> > > >
> > >
> > > I've pushed the whole set to my tmp.perf/cap branch, please chec
> >
> > Please hold on before moving further - I'm getting a segmentation
> > fault on ARM64 that I'm still trying to figure out.
>
> This is just sitting in my tmp branch, and in my local perf/core branch,
> so that I can test it with the containers, etc.
>
> Is this related to the following fix?
That is the first thing I thought about but no, it has nothing to do
with it. Patch 3/4 is where the problem shows up. The code in the
patch is fine, it is the repercussion it has on other part that needs
to be investigated.
Right now I see that kmap->ref_reloc_sym is NULL here [1] when tracing
with anything else than the 'u' option. I am currently investigating
the problem.
Igor, please see if you can reproduce on QEMU or an ARM64 based platform.
[1] https://elixir.bootlin.com/linux/v5.3-rc4/source/tools/perf/util/event.c#L945
>
> commit 3e70008a6021fffd2cd1614734603ea970773060
> Author: Leo Yan <leo.yan@linaro.org>
> Date: Fri Aug 9 18:47:52 2019 +0800
>
> perf trace: Fix segmentation fault when access syscall info on arm64
>
> 'perf trace' reports the segmentation fault as below on Arm64:
>
> # perf trace -e string -e augmented_raw_syscalls.c
> LLVM: dumping tools/perf/examples/bpf/augmented_raw_syscalls.o
> perf: Segmentation fault
> Obtained 12 stack frames.
> perf(sighandler_dump_stack+0x47) [0xaaaaac96ac87]
> linux-vdso.so.1(+0x5b7) [0xffffadbeb5b7]
> /lib/aarch64-linux-gnu/libc.so.6(strlen+0x10) [0xfffface7d5d0]
> /lib/aarch64-linux-gnu/libc.so.6(_IO_vfprintf+0x1ac7) [0xfffface49f97]
> /lib/aarch64-linux-gnu/libc.so.6(__vsnprintf_chk+0xc7) [0xffffacedfbe7]
> perf(scnprintf+0x97) [0xaaaaac9ca3ff]
> perf(+0x997bb) [0xaaaaac8e37bb]
> perf(cmd_trace+0x28e7) [0xaaaaac8ec09f]
> perf(+0xd4a13) [0xaaaaac91ea13]
> perf(main+0x62f) [0xaaaaac8a147f]
> /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0xe3) [0xfffface22d23]
> perf(+0x57723) [0xaaaaac8a1723]
> Segmentation fault
>
> This issue is introduced by commit 30a910d7d3e0 ("perf trace:
> Preallocate the syscall table"), it allocates trace->syscalls.table[]
> array and the element count is 'trace->sctbl->syscalls.nr_entries'; but
> on Arm64, the system call number is not continuously used; e.g. the
> syscall maximum id is 436 but the real entries is only 281.
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [tip:perf/core] perf ftrace: Use CAP_SYS_ADMIN instead of euid==0
2019-08-07 14:44 ` [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
2019-08-12 20:22 ` Arnaldo Carvalho de Melo
@ 2019-08-15 9:27 ` tip-bot for Igor Lubashev
1 sibling, 0 replies; 33+ messages in thread
From: tip-bot for Igor Lubashev @ 2019-08-15 9:27 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, jmorris, namhyung, alexander.shishkin, mathieu.poirier,
acme, hpa, alexey.budankov, linux-kernel, peterz, jolsa,
ilubashe, suzuki.poulose, tglx
Commit-ID: c766f3df635de14295e410c6dd5410bc416c24a0
Gitweb: https://git.kernel.org/tip/c766f3df635de14295e410c6dd5410bc416c24a0
Author: Igor Lubashev <ilubashe@akamai.com>
AuthorDate: Wed, 7 Aug 2019 10:44:17 -0400
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 14 Aug 2019 10:59:59 -0300
perf ftrace: Use CAP_SYS_ADMIN instead of euid==0
The 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>
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/bd8763b72ed4d58d0b42d44fbc7eb474d32e53a3.1565188228.git.ilubashe@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.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 20d4c0ce8b53..01a5bb58eb04 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;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v3 0/4] perf: Use capabilities instead of uid and euid
2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev
` (3 preceding siblings ...)
2019-08-07 14:44 ` [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev
@ 2019-08-12 9:13 ` Jiri Olsa
4 siblings, 0 replies; 33+ messages in thread
From: Jiri Olsa @ 2019-08-12 9:13 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 Wed, Aug 07, 2019 at 10:44:13AM -0400, Igor Lubashev wrote:
> 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 unprivileged 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 functions.
>
>
> Changelog:
> v3: * Fix arm64 compilation (thanks, Alexey and Jiri)
Acked-by: Jiri Olsa <jolsa@kernel.org>
thanks,
jirka
> 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 | 3 ++-
> 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(+), 10 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] 33+ messages in thread