[PATCH 0/3] perf: Allow running without stdin

2019-10-22 Thread Igor Lubashev
This series allows perf to run in batch mode with stdin closed.
This is arguably a bug fix for code that runs with --stdio option and does not
check for EOF return code from getc().

This series makes the following work as expected:

  $ perf top --stdio < /dev/null
  $ perf kvm top --stdio < /dev/null

Patches:
  01: perf top: Allow running without stdin
Make "perf top --stdio" handle EOF from stdin correctly.
There is one getc() that does not handle EOF explicitly, since its return
value is checked against a list of known characters, and the main loop in
display_thread() will handle the EOF on the next iteration.

  02: perf kvm: Allow running without stdin
Make "perf kvm --stdio" handle EOF from stdin correctly.

  03: perf kvm: Use evlist layer api when possible
This is a simple fix for a needless layering violation.

Igor Lubashev (3):
  perf top: Allow running without stdin
  perf kvm: Allow running without stdin
  perf kvm: Use evlist layer api when possible

 tools/perf/builtin-kvm.c | 35 +--
 tools/perf/builtin-top.c | 10 --
 2 files changed, 29 insertions(+), 16 deletions(-)

-- 
2.7.4



[PATCH 3/3] perf kvm: Use evlist layer api when possible

2019-10-22 Thread Igor Lubashev
No need for layer violations when a proper evlist api is available.

Signed-off-by: Igor Lubashev 
---
 tools/perf/builtin-kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 5217aa3596c7..340927c2b243 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1005,7 +1005,7 @@ static int kvm_events_live_report(struct perf_kvm_stat 
*kvm)
}
 
if (!rc && !done)
-   err = fdarray__poll(fda, 100);
+   err = evlist__poll(kvm->evlist, 100);
}
 
evlist__disable(kvm->evlist);
-- 
2.7.4



[PATCH 2/3] perf kvm: Allow running without stdin

2019-10-22 Thread Igor Lubashev
Allow perf kvm --stdio to run without access to stdin.
This lets perf kvm to run in a batch mode until interrupted.

The following now works as expected:

  $ perf kvm top --stdio < /dev/null

Signed-off-by: Igor Lubashev 
---
 tools/perf/builtin-kvm.c | 33 -
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 858da896b518..5217aa3596c7 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -930,18 +930,20 @@ static int fd_set_nonblock(int fd)
 
 static int perf_kvm__handle_stdin(void)
 {
-   int c;
-
-   c = getc(stdin);
-   if (c == 'q')
+   switch (getc(stdin)) {
+   case 'q':
+   done = 1;
return 1;
-
-   return 0;
+   case EOF:
+   return 0;
+   default:
+   return 1;
+   }
 }
 
 static int kvm_events_live_report(struct perf_kvm_stat *kvm)
 {
-   int nr_stdin, ret, err = -EINVAL;
+   int nr_stdin = -1, ret, err = -EINVAL;
struct termios save;
 
/* live flag must be set first */
@@ -972,13 +974,16 @@ static int kvm_events_live_report(struct perf_kvm_stat 
*kvm)
if (evlist__add_pollfd(kvm->evlist, kvm->timerfd) < 0)
goto out;
 
-   nr_stdin = evlist__add_pollfd(kvm->evlist, fileno(stdin));
-   if (nr_stdin < 0)
-   goto out;
-
if (fd_set_nonblock(fileno(stdin)) != 0)
goto out;
 
+   /* add stdin, if it is connected */
+   if (getc(stdin) != EOF) {
+   nr_stdin = evlist__add_pollfd(kvm->evlist, fileno(stdin));
+   if (nr_stdin < 0)
+   goto out;
+   }
+
/* everything is good - enable the events and process */
evlist__enable(kvm->evlist);
 
@@ -994,8 +999,10 @@ static int kvm_events_live_report(struct perf_kvm_stat 
*kvm)
if (err)
goto out;
 
-   if (fda->entries[nr_stdin].revents & POLLIN)
-   done = perf_kvm__handle_stdin();
+   if (nr_stdin >= 0 && fda->entries[nr_stdin].revents & POLLIN) {
+   if (!perf_kvm__handle_stdin())
+   fda->entries[nr_stdin].events = 0;
+   }
 
if (!rc && !done)
err = fdarray__poll(fda, 100);
-- 
2.7.4



[PATCH 1/3] perf top: Allow running without stdin

2019-10-22 Thread Igor Lubashev
Allow perf top --stdio to run without access to stdin.
This lets perf top to run in a batch mode until interrupted.

The following now works as expected:

  $ perf top < /dev/null

Signed-off-by: Igor Lubashev 
---
 tools/perf/builtin-top.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index d96f24c8770d..fbc0dc135b8a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -665,6 +665,7 @@ static void display_setup_sig(void)
 static void *display_thread(void *arg)
 {
struct pollfd stdin_poll = { .fd = 0, .events = POLLIN };
+   nfds_t nfds = 1;
struct termios save;
struct perf_top *top = arg;
int delay_msecs, c;
@@ -684,7 +685,8 @@ static void *display_thread(void *arg)
delay_msecs = top->delay_secs * MSEC_PER_SEC;
set_term_quiet_input();
/* trash return*/
-   getc(stdin);
+   if (getc(stdin) == EOF)
+   nfds = 0;
 
while (!done) {
perf_top__print_sym_table(top);
@@ -692,7 +694,7 @@ static void *display_thread(void *arg)
 * Either timeout expired or we got an EINTR due to SIGWINCH,
 * refresh screen in both cases.
 */
-   switch (poll(_poll, 1, delay_msecs)) {
+   switch (poll(_poll, nfds, delay_msecs)) {
case 0:
continue;
case -1:
@@ -701,6 +703,10 @@ static void *display_thread(void *arg)
__fallthrough;
default:
c = getc(stdin);
+   if (c == EOF) {
+   nfds = 0;
+   continue;
+   }
tcsetattr(0, TCSAFLUSH, );
 
if (perf_top__handle_keypress(top, c))
-- 
2.7.4



[tip: perf/core] perf tools: Warn that perf_event_paranoid can restrict kernel symbols

2019-08-29 Thread tip-bot2 for Igor Lubashev
The following commit has been merged into the perf/core branch of tip:

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

perf tools: Warn that perf_event_paranoid can restrict kernel symbols

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

Signed-off-by: Igor Lubashev 
Tested-by: Mathieu Poirier 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/1566869956-7154-6-git-send-email-iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/builtin-top.c| 2 +-
 tools/perf/builtin-trace.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

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


[tip: perf/core] perf symbols: Use CAP_SYSLOG with kptr_restrict checks

2019-08-29 Thread tip-bot2 for Igor Lubashev
The following commit has been merged into the perf/core branch of tip:

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

perf symbols: Use CAP_SYSLOG with kptr_restrict checks

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

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

Signed-off-by: Igor Lubashev 
Tested-by: Mathieu Poirier 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/1566869956-7154-5-git-send-email-iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/symbol.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4efde78..035f2e7 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -15,8 +16,10 @@
 #include 
 #include "annotate.h"
 #include "build-id.h"
+#include "cap.h"
 #include "util.h"
 #include "debug.h"
+#include "event.h"
 #include "machine.h"
 #include "map.h"
 #include "symbol.h"
@@ -2195,13 +2198,19 @@ static bool symbol__read_kptr_restrict(void)
char line[8];
 
if (fgets(line, sizeof(line), fp) != NULL)
-   value = ((geteuid() != 0) || (getuid() != 0)) ?
-   (atoi(line) != 0) :
-   (atoi(line) == 2);
+   value = perf_cap__capable(CAP_SYSLOG) ?
+   (atoi(line) >= 2) :
+   (atoi(line) != 0);
 
fclose(fp);
}
 
+   /* Per kernel/kallsyms.c:
+* we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+*/
+   if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+   value = true;
+
return value;
 }
 


[tip: perf/core] perf evsel: Kernel profiling is disallowed only when perf_event_paranoid > 1

2019-08-29 Thread tip-bot2 for Igor Lubashev
The following commit has been merged into the perf/core branch of tip:

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

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

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

Committer testing:

For a non-root user:

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

Before:

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

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

After:

When the kernel allows profiling the kernel in that scenario:

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

Reported-by: Arnaldo Carvalho de Melo 
Signed-off-by: Igor Lubashev 
Tested-by: Arnaldo Carvalho de Melo 
Tested-by: Mathieu Poirier 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/1566869956-7154-4-git-send-email-iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/evsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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


[tip: perf/core] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks

2019-08-29 Thread tip-bot2 for Igor Lubashev
The following commit has been merged into the perf/core branch of tip:

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

perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks

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

Signed-off-by: Arnaldo Carvalho de Melo 
Acked-by: Jiri Olsa 
Tested-by: Mathieu Poirier 
Reviewed-by: Mathieu Poirier  # coresight part
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/1566869956-7154-3-git-send-email-iluba...@akamai.com
Signed-off-by: Igor Lubashev 
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/arch/arm/util/cs-etm.c| 3 ++-
 tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
 tools/perf/arch/x86/util/intel-bts.c | 3 ++-
 tools/perf/arch/x86/util/intel-pt.c  | 2 +-
 tools/perf/util/evsel.c  | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c 
b/tools/perf/arch/arm/util/cs-etm.c
index a185dab..5d856ed 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -18,6 +18,7 @@
 #include "../../util/record.h"
 #include "../../util/auxtrace.h"
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evlist.h"
 #include "../../util/evsel.h"
 #include "../../util/pmu.h"
@@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record 
*itr,
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
struct evsel *evsel, *cs_etm_evsel = NULL;
struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+   bool privileged = perf_event_paranoid_check(-1);
int err = 0;
 
ptr->evlist = evlist;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c 
b/tools/perf/arch/arm64/util/arm-spe.c
index cdd5c0c..c7b38f0 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -12,6 +12,7 @@
 #include 
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -67,7 +68,7 @@ static int arm_spe_recording_options(struct auxtrace_record 
*itr,
container_of(itr, struct arm_spe_recording, itr);
struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
struct evsel *evsel, *arm_spe_evsel = NULL;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
struct evsel *tracking_evsel;
int err;
 
diff --git a/tools/perf/arch/x86/util/intel-bts.c 
b/tools/perf/arch/x86/util/intel-bts.c
index 1f2cf61..16d26ea 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -12,6 +12,7 @@
 #include 
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -108,7 +109,7 @@ static int intel_bts_recording_options(struct 
auxtrace_record *itr,
struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
struct evsel *evsel, *intel_bts_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
 
btsr->evlist = evlist;
btsr->snapshot_mode = opts->auxtrace_snapshot_mode;
diff --git a/tools/perf/arch/x86/util/intel-pt.c 
b/tools/perf/arch/x86/util/intel-pt.c
index 44cfe72..746981c 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -579,7 +579,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
bool have_timing_info, need_immediate = false;
struct evsel *evsel, *intel_pt_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
u64 tsc_bit;
int err;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index fa67635..7c704b8 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -282,7 +282,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr 
*attr, int idx)
 
 static bool perf_event_can_profile_kernel(void)
 {
-   return geteuid() == 0 || perf_event_paranoid() == -1;
+   return perf_event_paranoid_check(-1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)


[tip: perf/core] perf event: Check ref_reloc_sym before using it

2019-08-29 Thread tip-bot2 for Igor Lubashev
The following commit has been merged into the perf/core branch of tip:

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

perf event: Check ref_reloc_sym before using it

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

Reported-by: Mathieu Poirier 
Signed-off-by: Igor Lubashev 
Tested-by: Mathieu Poirier 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Jiri Olsa 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/1566869956-7154-2-git-send-email-iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/event.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

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


[PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it

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

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

===  Testing notes ===

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

I tested each permutation of:

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

  * 2 build environments: with and without libcap-dev

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

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

  * 2 users: root and non-root

Total: 336 permutations

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

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

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

=== Series Contents ===

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

  02: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
perf_event_paranoid level is verified.
* This patch has been reviewed previously and is unchanged.
* I kept Acks and Sign-offs.

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

  04: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
Replace the use of uid and euid with a check for CAP_SYSLOG when
kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).
* A previous version of this patch has been reviewed previously, but I
* modified it in a non-trivial way, so I removed Acks.

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

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

 tools/perf/arch/arm/util/cs-etm.c|  3 ++-
 tools/perf/arch/arm64/util/arm-spe.c |  3 ++-
 tools/perf/arch/x86/util/intel-bts.c |  3 ++-
 tools/perf/arch/x86/util/intel-pt.c  |  2 +-
 tools/perf/builtin-record.c  |  2 +-
 tools/perf/builtin-top.c |  2 +-
 tools/perf/builtin-trace.c   |  2 +-
 tools/perf/util/event.c  |  7 ---
 tools/perf/util/evsel.c  |  2 +-
 tools/perf/util/symbol.c | 15 ---
 10 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.7.4



[PATCH 1/5] perf event: Check ref_reloc_sym before using it

2019-08-26 Thread Igor Lubashev
Check for ref_reloc_sym before using it instead of checking
symbol_conf.kptr_restrict and relying solely on that check.

Signed-off-by: Igor Lubashev 
Reported-by: Mathieu Poirier 
---
 tools/perf/util/event.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

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



[PATCH 3/5] perf util: kernel profiling is disallowed only when perf_event_paranoid > 1

2019-08-26 Thread Igor Lubashev
Perf was too restrictive about sysctl kernel.perf_event_paranoid. The
kernel only disallows profiling when perf_event_paranoid > 1. Make perf do
the same.

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

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



[PATCH 2/5] perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks

2019-08-26 Thread 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 
Acked-by: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Mathieu Poirier 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/ad56df5452eeafb99dda9fc3d30f0f487aace503.1565188228.git.iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 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 
 
 #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 
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct 
auxtrace_record *itr,
struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
struct evsel *evsel, *intel_bts_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
 
btsr->evlist = evlist;
btsr->snapshot_mode = opts->auxtrace_snapshot_mode;
diff --git a/tools/perf/arch/x86/util/intel-pt.c 
b/tools/perf/arch/x86/util/intel-pt.c
index a8e633aa278a..7abccc0b0dc0 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -578,7 +578,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
bool have_timing_info, need_immediate = false;
struct evsel *evsel, *intel_pt_evsel = NULL;
const struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
u64 tsc_bit;
int err;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0a33f7322ecc..0b3b5af33954 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -279,7 +279,7 @@ struct evsel *perf_evsel__new_idx(struct perf_event_attr 
*attr, int idx)
 
 static bool perf_event_can_profile_kernel(void)
 {
-   return geteuid() == 0 || perf_event_paranoid() == -1;
+   return perf_event_paranoid_check(-1);
 }
 
 struct evsel *perf_evsel__new_cycles(bool precise)
-- 
2.7.4



[PATCH 5/5] perf: warn that perf_event_paranoid can restrict kernel symbols

2019-08-26 Thread Igor Lubashev
Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
symbols.

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

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



[PATCH 4/5] perf symbols: Use CAP_SYSLOG with kptr_restrict checks

2019-08-26 Thread Igor Lubashev
The kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0
when checking kptr_restrict. Make perf do the same.

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

Signed-off-by: Igor Lubashev 
Cc: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Mathieu Poirier 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/291d2cda6ee75b4cd4c9ce717c177db18bf03a31.1565188228.git.iluba...@akamai.com
Cc: Arnaldo Carvalho de Melo 
---
 tools/perf/util/symbol.c | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4efde7879474..035f2e75728c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -15,8 +16,10 @@
 #include 
 #include "annotate.h"
 #include "build-id.h"
+#include "cap.h"
 #include "util.h"
 #include "debug.h"
+#include "event.h"
 #include "machine.h"
 #include "map.h"
 #include "symbol.h"
@@ -2195,13 +2198,19 @@ static bool symbol__read_kptr_restrict(void)
char line[8];
 
if (fgets(line, sizeof(line), fp) != NULL)
-   value = ((geteuid() != 0) || (getuid() != 0)) ?
-   (atoi(line) != 0) :
-   (atoi(line) == 2);
+   value = perf_cap__capable(CAP_SYSLOG) ?
+   (atoi(line) >= 2) :
+   (atoi(line) != 0);
 
fclose(fp);
}
 
+   /* Per kernel/kallsyms.c:
+* we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+*/
+   if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+   value = true;
+
return value;
 }
 
-- 
2.7.4



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

2019-08-15 Thread tip-bot for Igor Lubashev
Commit-ID:  c766f3df635de14295e410c6dd5410bc416c24a0
Gitweb: https://git.kernel.org/tip/c766f3df635de14295e410c6dd5410bc416c24a0
Author: Igor Lubashev 
AuthorDate: Wed, 7 Aug 2019 10:44:17 -0400
Committer:  Arnaldo Carvalho de Melo 
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 
Acked-by: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Mathieu Poirier 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
Link: 
http://lkml.kernel.org/r/bd8763b72ed4d58d0b42d44fbc7eb474d32e53a3.1565188228.git.iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 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 
 #include 
 #include 
+#include 
 
 #include "debug.h"
 #include 
@@ -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;
}


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

2019-08-15 Thread tip-bot for Igor Lubashev
Commit-ID:  c22e150e3afa6f8db2300bd510e4ac26bbee1bf3
Gitweb: https://git.kernel.org/tip/c22e150e3afa6f8db2300bd510e4ac26bbee1bf3
Author: Igor Lubashev 
AuthorDate: Wed, 7 Aug 2019 10:44:14 -0400
Committer:  Arnaldo Carvalho de Melo 
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 "", line 1, in 
  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 (0x7f724a4ef000)
  $

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 
Acked-by: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Mathieu Poirier 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
Cc: linux-arm-ker...@lists.infradead.org
[ split from a larger patch ]
Link: 
http://lkml.kernel.org/r/8a1e76cf5c7c9796d0d4d240fbaa85305298aafa.1565188228.git.iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 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 ..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 
+#include 
+
+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, ) != 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 ..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 
+#include 
+#include 
+
+#ifdef HAVE_LIBCAP_SUPPORT
+
+#include 
+
+bool perf_cap__capable(cap_value_t cap);
+
+#else
+
+#include 
+#include 
+
+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 10064

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

2019-08-15 Thread tip-bot for Igor Lubashev
Commit-ID:  74d5f3d06f707eb5f7e1908ad88954bde02000ce
Gitweb: https://git.kernel.org/tip/74d5f3d06f707eb5f7e1908ad88954bde02000ce
Author: Igor Lubashev 
AuthorDate: Wed, 7 Aug 2019 10:44:14 -0400
Committer:  Arnaldo Carvalho de Melo 
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:
  
  ...libbfd: [ on  ]
  ...libcap: [ OFF ]
  ...libelf: [ on  ]
  
  Makefile.config:833: No libcap found, disables capability support, please 
install libcap-devel/libcap-dev
  
  $ 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 
|  ^~
  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:
  
  ...libbfd: [ on  ]
  ...libcap: [ on  ]
  ...libelf: [ on  ]
  >
CC   /tmp/build/perf/jvmti/libjvmti.o
  >
  $ 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 (0x7ffc35bfe000)
libcap.so.2 => /lib64/libcap.so.2 (0x7ff9c62ff000)
libc.so.6 => /lib64/libc.so.6 (0x7ff9c6139000)
/lib64/ld-linux-x86-64.so.2 (0x7ff9c6326000)
  $

Signed-off-by: Igor Lubashev 
Acked-by: Jiri Olsa 
Tested-by: Arnaldo Carvalho de Melo 
Cc: Alexander Shishkin 
Cc: Alexey Budankov 
Cc: James Morris 
Cc: Mathieu Poirier 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Suzuki Poulouse 
[ split from a larger patch ]
Link: 
http://lkml.kernel.org/r/8a1e76cf5c7c9796d0d4d240fbaa85305298aafa.1565188228.git.iluba...@akamai.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 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/b

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

2019-08-07 Thread 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 
---
 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 ..d2a2e152195f
--- /dev/null
+++ b/tools/build/feature/test-libcap.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+#include 
+#include 
+
+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, ) != 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 ..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 
+#include 
+
+bool perf_ca

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

2019-08-07 Thread Igor Lubashev
Series v1: 
https://lkml.kernel.org/lkml/1562112605-6235-1-git-send-email-iluba...@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 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

2019-08-07 Thread 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 
---
 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 
 
 #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 
 
 #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



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

2019-08-07 Thread 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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -15,8 +16,10 @@
 #include 
 #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



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

2019-08-07 Thread 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 
---
 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 
 #include 
 #include 
+#include 
 
 #include "debug.h"
 #include 
@@ -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



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

2019-08-06 Thread 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 
---
 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 
 #include 
 #include 
+#include 
 
 #include "debug.h"
 #include 
@@ -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



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

2019-08-06 Thread Igor Lubashev
Series v1: 
https://lkml.kernel.org/lkml/1562112605-6235-1-git-send-email-iluba...@akamai.com


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

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

The series consists of four patches:

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

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

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

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

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

   kernel.perf_event_paranoid=2
   kernel.kptr_restrict=1

As an unpriviledged user who is in perf_users group (setup via instructions
above), I executed:
   perf record -a -- sleep 1

Without the patch, perf record did not capture any kernel functions.
With the patch, perf included all kernel funcitons.


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


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

 tools/build/Makefile.feature |  2 ++
 tools/build/feature/Makefile |  4 
 tools/build/feature/test-libcap.c| 20 
 tools/perf/Makefile.config   | 11 +++
 tools/perf/Makefile.perf |  2 ++
 tools/perf/arch/arm/util/cs-etm.c|  3 ++-
 tools/perf/arch/arm64/util/arm-spe.c |  4 ++--
 tools/perf/arch/x86/util/intel-bts.c |  3 ++-
 tools/perf/arch/x86/util/intel-pt.c  |  2 +-
 tools/perf/builtin-ftrace.c  |  4 +++-
 tools/perf/util/Build|  2 ++
 tools/perf/util/cap.c| 29 +
 tools/perf/util/cap.h| 24 
 tools/perf/util/event.h  |  1 +
 tools/perf/util/evsel.c  |  2 +-
 tools/perf/util/python-ext-sources   |  1 +
 tools/perf/util/symbol.c | 15 +++
 tools/perf/util/util.c   |  9 +
 18 files changed, 127 insertions(+), 11 deletions(-)
 create mode 100644 tools/build/feature/test-libcap.c
 create mode 100644 tools/perf/util/cap.c
 create mode 100644 tools/perf/util/cap.h

-- 
2.7.4



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

2019-08-06 Thread 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 
---
 tools/perf/arch/arm/util/cs-etm.c| 3 ++-
 tools/perf/arch/arm64/util/arm-spe.c | 4 ++--
 tools/perf/arch/x86/util/intel-bts.c | 3 ++-
 tools/perf/arch/x86/util/intel-pt.c  | 2 +-
 tools/perf/util/evsel.c  | 2 +-
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c 
b/tools/perf/arch/arm/util/cs-etm.c
index 5cb07e8cb296..b87a1ca2968f 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -18,6 +18,7 @@
 #include "../../perf.h"
 #include "../../util/auxtrace.h"
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evlist.h"
 #include "../../util/evsel.h"
 #include "../../util/pmu.h"
@@ -254,7 +255,7 @@ static int cs_etm_recording_options(struct auxtrace_record 
*itr,
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
struct evsel *evsel, *cs_etm_evsel = NULL;
struct perf_cpu_map *cpus = evlist->core.cpus;
-   bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+   bool privileged = perf_event_paranoid_check(-1);
int err = 0;
 
ptr->evlist = evlist;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c 
b/tools/perf/arch/arm64/util/arm-spe.c
index 00915b8fd05b..200bc973371b 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -12,6 +12,7 @@
 #include 
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -65,8 +66,7 @@ static int arm_spe_recording_options(struct auxtrace_record 
*itr,
struct arm_spe_recording *sper =
container_of(itr, struct arm_spe_recording, itr);
struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu;
-   struct evsel *evsel, *arm_spe_evsel = NULL;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
struct evsel *tracking_evsel;
int err;
 
diff --git a/tools/perf/arch/x86/util/intel-bts.c 
b/tools/perf/arch/x86/util/intel-bts.c
index 7b23318ebd7b..56a76142e9fd 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -12,6 +12,7 @@
 #include 
 
 #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



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

2019-08-06 Thread 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 
---
 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 ..d2a2e152195f
--- /dev/null
+++ b/tools/build/feature/test-libcap.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+#include 
+#include 
+
+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, ) != 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 ..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 
+#include 
+
+bool perf_ca

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

2019-08-06 Thread 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 
---
 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 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -15,8 +16,10 @@
 #include 
 #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



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

2019-07-17 Thread Igor Lubashev
Kernel requires CAP_SYS_ADMIN instead of euid==0 to mount debugfs for ftrace.
Make perf require the same.

Signed-off-by: Igor Lubashev 
---
 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 9c228c55e1fb..2f632b11ebba 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "debug.h"
 #include 
@@ -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



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

2019-07-02 Thread Igor Lubashev
Kernel is using CAP_SYSLOG capcbility 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 
---
 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 5cbad55cd99d..fd68dae3f58e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -15,8 +16,10 @@
 #include 
 #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"
@@ -889,7 +892,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) {
@@ -2100,9 +2107,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



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

2019-07-02 Thread Igor Lubashev
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 three patches:

  01: perf: Add capability-related utilities
Add utility functions to check capabilities and perf_event_paranoid checks.

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

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

   kernel.perf_event_paranoid=2
   kernel.kptr_restrict=1

As an unpriviledged user who is in perf_users group (setup via instructions
above), I executed:
   perf record -a -- sleep 1

Without the patch, perf record did not capture any kernel functions.
With the patch, perf included all kernel funcitons.

Igor Lubashev (3):
  perf: Add capability-related utilities
  perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
  perf: Use CAP_SYSLOG with kptr_restrict checks

 tools/perf/Makefile.config   |  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/util/Build|  1 +
 tools/perf/util/cap.c| 24 
 tools/perf/util/cap.h| 10 ++
 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 +
 13 files changed, 66 insertions(+), 10 deletions(-)
 create mode 100644 tools/perf/util/cap.c
 create mode 100644 tools/perf/util/cap.h

-- 
2.7.4



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

2019-07-02 Thread Igor Lubashev
Add utilities to help checking capabilities of the running process.
Make perf link with libcap.

Signed-off-by: Igor Lubashev 
---
 tools/perf/Makefile.config |  2 +-
 tools/perf/util/Build  |  1 +
 tools/perf/util/cap.c  | 24 
 tools/perf/util/cap.h  | 10 ++
 tools/perf/util/event.h|  1 +
 tools/perf/util/python-ext-sources |  1 +
 tools/perf/util/util.c |  9 +
 7 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 tools/perf/util/cap.c
 create mode 100644 tools/perf/util/cap.h

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 85fbcd265351..21470a50ed39 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -259,7 +259,7 @@ CXXFLAGS += -Wno-strict-aliasing
 # adding assembler files missing the .GNU-stack linker note.
 LDFLAGS += -Wl,-z,noexecstack
 
-EXTLIBS = -lpthread -lrt -lm -ldl
+EXTLIBS = -lpthread -lrt -lm -ldl -lcap
 
 ifeq ($(FEATURES_DUMP),)
 include $(srctree)/tools/build/Makefile.feature
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 6d5bbc8b589b..9cc6e9b34ebd 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -1,6 +1,7 @@
 perf-y += annotate.o
 perf-y += block-range.o
 perf-y += build-id.o
+perf-y += cap.o
 perf-y += config.o
 perf-y += ctype.o
 perf-y += db-export.o
diff --git a/tools/perf/util/cap.c b/tools/perf/util/cap.c
new file mode 100644
index ..c42ea32663cf
--- /dev/null
+++ b/tools/perf/util/cap.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Capability utilities
+ */
+#include "cap.h"
+#include 
+#include 
+
+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, ) != 0)
+   val = CAP_CLEAR;
+
+   if (cap_free(caps) != 0)
+   return false;
+
+   return val == CAP_SET;
+}
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
new file mode 100644
index ..5521de78b228
--- /dev/null
+++ b/tools/perf/util/cap.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_CAP_H
+#define __PERF_CAP_H
+
+#include 
+#include 
+
+bool perf_cap__capable(cap_value_t cap);
+
+#endif /* __PERF_CAP_H */
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 9e999550f247..013d9e28fcac 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -849,6 +849,7 @@ void  cpu_map_data__synthesize(struct cpu_map_data *data, 
struct cpu_map *map,
 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 7aa0ea64544e..4545eaf018b5 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -6,6 +6,7 @@
 #
 
 util/python.c
+util/cap.c
 util/ctype.c
 util/evlist.c
 util/evsel.c
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index d388f80d8703..cde538ec727d 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -16,10 +16,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include "cap.h"
 #include "strlist.h"
 #include "string2.h"
 
@@ -456,6 +458,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



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

2019-07-02 Thread 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 
---
 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 911426721170..e004ba7ad957 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -17,6 +17,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"
@@ -106,7 +107,7 @@ static int cs_etm_recording_options(struct auxtrace_record 
*itr,
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
struct perf_evsel *evsel, *cs_etm_evsel = NULL;
const struct cpu_map *cpus = evlist->cpus;
-   bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0);
+   bool privileged = perf_event_paranoid_check(-1);
 
ptr->evlist = evlist;
ptr->snapshot_mode = opts->auxtrace_snapshot_mode;
diff --git a/tools/perf/arch/arm64/util/arm-spe.c 
b/tools/perf/arch/arm64/util/arm-spe.c
index 5ccfce87e693..f5ec6953c69c 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -11,6 +11,7 @@
 #include 
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -65,7 +66,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 perf_evsel *evsel, *arm_spe_evsel = NULL;
-   bool privileged = geteuid() == 0 || perf_event_paranoid() < 0;
+   bool privileged = perf_event_paranoid_check(-1);
struct perf_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 e6d4d9591c79..fe7cecdb494d 100644
--- a/tools/perf/arch/x86/util/intel-bts.c
+++ b/tools/perf/arch/x86/util/intel-bts.c
@@ -11,6 +11,7 @@
 #include 
 
 #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 perf_evsel *evsel, *intel_bts_evsel = NULL;
const struct cpu_map *cpus = evlist->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 1869f62a10cd..44d2194fdab3 100644
--- a/tools/perf/arch/x86/util/intel-pt.c
+++ b/tools/perf/arch/x86/util/intel-pt.c
@@ -557,7 +557,7 @@ static int intel_pt_recording_options(struct 
auxtrace_record *itr,
bool have_timing_info, need_immediate = false;
struct perf_evsel *evsel, *intel_pt_evsel = NULL;
const struct cpu_map *cpus = evlist->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 4a5947625c5c..ce28d890d6bf 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -277,7 +277,7 @@ struct perf_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 perf_evsel *perf_evsel__new_cycles(bool precise)
-- 
2.7.4



[RFC PATCH 1/1] security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

2019-06-13 Thread Igor Lubashev
Many kernel interfaces require real and/or effective root uid instead
of relying solely of capabilities. An executable that uses such
interfaces has to be set-uid-root or be executed by a thread with
effective root uid. Presently, fsuid and saved uid will reset to the
effective uid during execve. As a result, it is not possible to
execute a binary such that it has effective root uid but retains
fsuid/fsgid of the actual user. Retaining fsuid/fsgid of the actual
user could be required if the executable needs to access the
filesystem. It may also be desired from the security perspective of
delegating the minimal set of privileges.

Setting SECURE_KEEP_FSUID bit ensures that the current fsuid/fsgiud is
retained by execve.

Signed-off-by: Igor Lubashev 
---
 include/uapi/linux/securebits.h | 10 +-
 security/commoncap.c|  9 +++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d98877ff1a..6c20b2287d6f 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,18 @@
 #define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
 
+/* When set, execve does not reset fsuid/fsgid to euid/egid */
+#define SECURE_KEEP_FSUID  8
+#define SECURE_KEEP_FSUID_LOCKED   9  /* make bit-8 immutable */
+
+#define SECBIT_KEEP_FSUID   (issecure_mask(SECURE_KEEP_FSUID))
+#define SECBIT_KEEP_FSUID_LOCKED (issecure_mask(SECURE_KEEP_FSUID_LOCKED))
+
 #define SECURE_ALL_BITS(issecure_mask(SECURE_NOROOT) | \
 issecure_mask(SECURE_NO_SETUID_FIXUP) | \
 issecure_mask(SECURE_KEEP_CAPS) | \
-issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+issecure_mask(SECURE_KEEP_FSUID))
 #define SECURE_ALL_LOCKS   (SECURE_ALL_BITS << 1)
 
 #endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index c0b9664ee49e..e4de823a1d4e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -847,8 +847,13 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
   old->cap_permitted);
}
 
-   new->suid = new->fsuid = new->euid;
-   new->sgid = new->fsgid = new->egid;
+   new->suid = new->euid;
+   new->sgid = new->egid;
+
+   if (!issecure(SECURE_KEEP_FSUID)) {
+   new->fsuid = new->euid;
+   new->fsgid = new->egid;
+   }
 
/* File caps or setid cancels ambient. */
if (has_fcap || is_setid)
-- 
2.7.4



[RFC PATCH 0/1] security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

2019-06-13 Thread Igor Lubashev
I've posted this in March but received no response. Reposting.

This patch introduces SECURE_KEEP_FSUID to allow fsuid/fsgid to be
preserved across execve. It is currently impossible to execve a
program such that effective and filesystem uid differ.

The need for this functionality arose from a desire to allow certain
non-privileged users to run perf. To do this, we install perf without
set-uid-root and have a set-uid-root wrapper decide who is allowed to
run perf (and with what arguments).

The wrapper must execve perf with real and effective root uid, because
perf and KASLR require this. However, that presently resets fsuid to
root, giving the user ability to read and overwrite any file owned by
root (perf report -i, perf record -o). Also, perf record will create
perf.data that cannot be deleted by the user.

We cannot reset /proc/sys/kernel/perf_event_paranoid to a permissive
level, since we must be selective which users have the permissions.

Of course, we could fix our problem by a patch to perf to allow
passing a username on the command line and having perf execute
setfsuid before opening files. However, perf is not the only program
that uses kernel features that require root uid/euid, so a general
solution that does not involve updating all such programs seems
warranted.

I will update man pages, if this patch is deemed a good idea.

Igor Lubashev (1):
  security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

 include/uapi/linux/securebits.h | 10 +-
 security/commoncap.c|  9 +++--
 2 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.7.4



[PATCH 1/1] RFC: security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

2019-03-25 Thread Igor Lubashev
Many kernel interfaces require real and/or effective root uid instead
of relying solely of capabilities. An executable that uses such
interfaces has to be set-uid-root or be executed by a thread with
effective root uid. Presently, fsuid and saved uid will reset to the
effective uid during execve. As a result, it is not possible to
execute a binary such that it has effective root uid but retains
fsuid/fsgid of the actual user. Retaining fsuid/fsgid of the actual
user could be required if the executable needs to access the
filesystem. It may also be desired from the security perspective of
delegating the minimal set of privileges.

Setting SECURE_KEEP_FSUID bit ensures that the current fsuid/fsgiud is
retained by execve.

Signed-off-by: Igor Lubashev 
---
 include/uapi/linux/securebits.h | 10 +-
 security/commoncap.c|  9 +++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/securebits.h b/include/uapi/linux/securebits.h
index d6d9887..6c20b22 100644
--- a/include/uapi/linux/securebits.h
+++ b/include/uapi/linux/securebits.h
@@ -52,10 +52,18 @@
 #define SECBIT_NO_CAP_AMBIENT_RAISE_LOCKED \
(issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE_LOCKED))
 
+/* When set, execve does not reset fsuid/fsgid to euid/egid */
+#define SECURE_KEEP_FSUID  8
+#define SECURE_KEEP_FSUID_LOCKED   9  /* make bit-8 immutable */
+
+#define SECBIT_KEEP_FSUID   (issecure_mask(SECURE_KEEP_FSUID))
+#define SECBIT_KEEP_FSUID_LOCKED (issecure_mask(SECURE_KEEP_FSUID_LOCKED))
+
 #define SECURE_ALL_BITS(issecure_mask(SECURE_NOROOT) | \
 issecure_mask(SECURE_NO_SETUID_FIXUP) | \
 issecure_mask(SECURE_KEEP_CAPS) | \
-issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE))
+issecure_mask(SECURE_NO_CAP_AMBIENT_RAISE) | \
+issecure_mask(SECURE_KEEP_FSUID))
 #define SECURE_ALL_LOCKS   (SECURE_ALL_BITS << 1)
 
 #endif /* _UAPI_LINUX_SECUREBITS_H */
diff --git a/security/commoncap.c b/security/commoncap.c
index c477fb6..490a7b4 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -852,8 +852,13 @@ int cap_bprm_set_creds(struct linux_binprm *bprm)
   old->cap_permitted);
}
 
-   new->suid = new->fsuid = new->euid;
-   new->sgid = new->fsgid = new->egid;
+   new->suid = new->euid;
+   new->sgid = new->egid;
+
+   if (!issecure(SECURE_KEEP_FSUID)) {
+   new->fsuid = new->euid;
+   new->fsgid = new->egid;
+   }
 
/* File caps or setid cancels ambient. */
if (has_fcap || is_setid)
-- 
2.7.4



[PATCH 0/1] RFC: security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

2019-03-25 Thread Igor Lubashev
This patch introduces SECURE_KEEP_FSUID to allow fsuid/fsgid to be
preserved across execve. I ran into a need for a patch trying to
implement a set-uid-root wrapper for perf.

My set-uid-root wrapper implements local policies, allowing only
certain users to run perf and only with certain arguments.

Perf, like a number of other kernel features, checks euid (and KASLR
access, required for perf top and perf report, also checks real uid)
in addition to checking capabilities.  Hence, I must execve perf from
my wrapper with root euid.

However, when I execve perf with root euid, it automatically obtains
root fsuid. This is very undesirable for a number of reasons:

1. 'perf record' will create perf.data file that cannot be deleted by
   the user.

2. 'perf record' becomes insecure, allowing the user an ability to
   overwrite any key file owned by root (and because of
   time-of-check/time-of-use principle, nothing I can check in the
   wrapper can reliably prevent the user from doing so).

3. 'perf report' can potentially read files that the user does not
   have permissions to read.


Perf and KASLR are not the only kernel features that check for root
uid/euid, so a general approach like the one in this patch seems
warranted.


This patch is the minimal set of changes required to achieve my goals.
However, I am wondering if we might want to go a bit further and have
a secure bit that stops fsuid/fsgid following euid/egid in all
contexts, including set*uid as well as ignoring uid/suid/euid in
setfsuid (and similarly for set*gid and setfsgid).

I will update man pages as needed.

Igor Lubashev (1):
  security: add SECURE_KEEP_FSUID to preserve fsuid/fsgid across execve

 include/uapi/linux/securebits.h | 10 +-
 security/commoncap.c|  9 +++--
 2 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.7.4