[PATCH v3 0/4] perf: Use capabilities instead of uid and euid

[PATCH v3 0/4] perf: Use capabilities instead of uid and euid

[Igor Lubashev]

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


Kernel is using capabilities instead of uid and euid to restrict access to
kernel pointers and tracing facilities.  This patch series updates the perf to
better match the security model used by the kernel.

This series enables instructions in Documentation/admin-guide/perf-security.rst
to actually work, even when kernel.perf_event_paranoid=2 and
kernel.kptr_restrict=1.

The series consists of four patches:

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

  02: perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
    Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
    perf_event_paranoid level is verified.

  03: perf: Use CAP_SYSLOG with kptr_restrict checks
    Replace the use of uid and euid with a check for CAP_SYSLOG when
    kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
    Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).

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

I tested this by following Documentation/admin-guide/perf-security.rst
guidelines and setting sysctls:

   kernel.perf_event_paranoid=2
   kernel.kptr_restrict=1

As an 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)
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

[PATCH v3 1/4] perf: Add capability-related utilities

[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

Re: [PATCH v3 1/4] perf: Add capability-related utilities

[Arnaldo Carvalho de Melo]

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>

[tip:perf/core] tools build: Add capability-related feature detection

[tip-bot for Igor Lubashev]

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

[tip:perf/core] perf tools: Add helpers to use capabilities if present

[tip-bot for Igor Lubashev]

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

[PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

[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

Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

[Arnaldo Carvalho de Melo]

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

RE: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

[Lubashev, Igor]

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

Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

[Arnaldo Carvalho de Melo]

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

[PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Mathieu Poirier]

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
>

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Arnaldo Carvalho de Melo]

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

RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Lubashev, Igor]

> 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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Mathieu Poirier]

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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Mathieu Poirier]

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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Mathieu Poirier]

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

RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Lubashev, Igor]

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

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Mathieu Poirier]

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
>

Re: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Arnaldo Carvalho de Melo]

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

RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Lubashev, Igor]

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

RE: [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks

[Lubashev, Igor]

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

[PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[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

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Arnaldo Carvalho de Melo]

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;
 	}
 

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Mathieu Poirier]

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

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Arnaldo Carvalho de Melo]

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.

Re: [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace

[Mathieu Poirier]

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

[tip:perf/core] perf ftrace: Use CAP_SYS_ADMIN instead of euid==0

[tip-bot for Igor Lubashev]

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;
 	}

Re: [PATCH v3 0/4] perf: Use capabilities instead of uid and euid

[Jiri Olsa]

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
>