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

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

[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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Igor Lubashev]

Add utilities to help checking capabilities of the running process.
Make perf link with libcap.

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 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 000000000000..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 <stdbool.h>
+#include <sys/capability.h>
+
+bool perf_cap__capable(cap_value_t cap)
+{
+	cap_flag_value_t val;
+	cap_t caps = cap_get_proc();
+
+	if (!caps)
+		return false;
+
+	if (cap_get_flag(caps, cap, CAP_EFFECTIVE, &val) != 0)
+		val = CAP_CLEAR;
+
+	if (cap_free(caps) != 0)
+		return false;
+
+	return val == CAP_SET;
+}
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
new file mode 100644
index 000000000000..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 <stdbool.h>
+#include <sys/capability.h>
+
+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 <string.h>
 #include <errno.h>
 #include <limits.h>
+#include <linux/capability.h>
 #include <linux/kernel.h>
 #include <linux/log2.h>
 #include <linux/time64.h>
 #include <unistd.h>
+#include "cap.h"
 #include "strlist.h"
 #include "string2.h"
 
@@ -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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jiri Olsa]

On Tue, Jul 02, 2019 at 08:10:03PM -0400, Igor Lubashev wrote:
> Add utilities to help checking capabilities of the running process.
> Make perf link with libcap.
> 
> Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> ---
>  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

I wonder we should detect libcap or it's everywhere.. Arnaldo's compile test suite might tell

jirka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Arnaldo Carvalho de Melo]

Em Tue, Jul 16, 2019 at 10:46:43AM +0200, Jiri Olsa escreveu:
> On Tue, Jul 02, 2019 at 08:10:03PM -0400, Igor Lubashev wrote:
> > Add utilities to help checking capabilities of the running process.
> > Make perf link with libcap.
> > 
> > Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> > ---
> >  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
> 
> I wonder we should detect libcap or it's everywhere.. Arnaldo's compile test suite might tell

I'll add this tentatively and try to build it in my test suite.

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Arnaldo Carvalho de Melo]

Em Wed, Jul 17, 2019 at 06:05:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jul 16, 2019 at 10:46:43AM +0200, Jiri Olsa escreveu:
> > On Tue, Jul 02, 2019 at 08:10:03PM -0400, Igor Lubashev wrote:
> > > Add utilities to help checking capabilities of the running process.
> > > Make perf link with libcap.
> > > 
> > > Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> > > ---
> > >  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
> > 
> > I wonder we should detect libcap or it's everywhere.. Arnaldo's compile test suite might tell
> 
> I'll add this tentatively and try to build it in my test suite.

So, not even in my notebook this worked straight away:

  CC       /tmp/build/perf/util/cap.o
  CC       /tmp/build/perf/util/config.o
In file included from util/cap.c:5:
util/cap.h:6:10: fatal error: sys/capability.h: No such file or directory
    6 | #include <sys/capability.h>
      |          ^~~~~~~~~~~~~~~~~~
compilation terminated.
mv: cannot stat '/tmp/build/perf/util/.cap.o.tmp': No such file or directory


I had to first do:

dnf install libcap-devel

So we need to have a feature test and fail if that is not installed,
i.e. libcap becomes a hard req for building perf, which I think is
reasonable, one more shouldn't hurt, right?

With all the features enabled:

[acme@quaco perf]$ ldd ~/bin/perf
	linux-vdso.so.1 (0x00007ffe7278a000)
	libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8 (0x00007f7be52f1000)
	libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f7be52d7000)
	liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f7be52ae000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f7be528d000)
	librt.so.1 => /lib64/librt.so.1 (0x00007f7be5283000)
	libm.so.6 => /lib64/libm.so.6 (0x00007f7be513d000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007f7be5135000)
	libcap.so.2 => /lib64/libcap.so.2 (0x00007f7be512e000)
	libelf.so.1 => /lib64/libelf.so.1 (0x00007f7be5113000)
	libdw.so.1 => /lib64/libdw.so.1 (0x00007f7be50c0000)
	libslang.so.2 => /lib64/libslang.so.2 (0x00007f7be4de8000)
	libperl.so.5.28 => /lib64/libperl.so.5.28 (0x00007f7be4ac2000)
	libc.so.6 => /lib64/libc.so.6 (0x00007f7be48fa000)
	libpython2.7.so.1.0 => /lib64/libpython2.7.so.1.0 (0x00007f7be4690000)
	libz.so.1 => /lib64/libz.so.1 (0x00007f7be4676000)
	libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f7be45d1000)
	libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f7be45c3000)
	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007f7be456d000)
	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f7be4551000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f7be5331000)
	libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f7be453d000)
	libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f7be4502000)
	libutil.so.1 => /lib64/libutil.so.1 (0x00007f7be44fd000)
	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007f7be44ed000)
	libpopt.so.0 => /lib64/libpopt.so.0 (0x00007f7be44dd000)
	libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f7be44d3000)
	libgmodule-2.0.so.0 => /lib64/libgmodule-2.0.so.0 (0x00007f7be44cd000)
	libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007f7be43a9000)
	libpcre.so.1 => /lib64/libpcre.so.1 (0x00007f7be4335000)
[acme@quaco perf]$

;-)

So, please check tools/build/feature/ and check how this is done and add
a test and the warning in tools/perf/Makefile.config so that we get an
error message stating that libcap-dev or libcap-devel should be
installed.

I'll do it if there is any difficulty, just not right now as I'm busy
and want to get a pull req out of the door.

- Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Arnaldo Carvalho de Melo]

Em Wed, Jul 17, 2019 at 08:46:52PM -0300, Arnaldo Carvalho de Melo escreveu:
> I'll do it if there is any difficulty, just not right now as I'm busy
> and want to get a pull req out of the door.

Also please find the first patch fixed up wrt a conflict with the
pythong binding, please use it instead as that is what applies to my
current perf/core branch.

It has the ack from Alexey and one I think Jiri would provide, judging
from his positive tone to the patches :)

- Arnaldo

commit 8048a0884a3f98bae2434d141711d72382b784b0
Author: Igor Lubashev <ilubashe@akamai.com>
Date:   Wed Jul 17 20:39:03 2019 -0300

    perf tools: Add capability-related utilities
    
    Add utilities to help checking capabilities of the running process.
    Make perf link with libcap.
    
    Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
    Acked-by: Alexey Budankov <alexey.budankov@linux.intel.com>
    Acked-by: Jiri Olsa <jolsa@kernel.org>
    CC: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: James Morris <jmorris@namei.org>
    Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
    Link: https://lkml.kernel.org/r/1562112605-6235-2-git-send-email-ilubashe@akamai.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 89ac5a1f1550..b9cf084f32d7 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 14f812bb07a7..61ed1a3005d4 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 000000000000..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 <stdbool.h>
+#include <sys/capability.h>
+
+bool perf_cap__capable(cap_value_t cap)
+{
+	cap_flag_value_t val;
+	cap_t caps = cap_get_proc();
+
+	if (!caps)
+		return false;
+
+	if (cap_get_flag(caps, cap, CAP_EFFECTIVE, &val) != 0)
+		val = CAP_CLEAR;
+
+	if (cap_free(caps) != 0)
+		return false;
+
+	return val == CAP_SET;
+}
diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h
new file mode 100644
index 000000000000..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 <stdbool.h>
+#include <sys/capability.h>
+
+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 1f1da6082806..b4128f72f2e8 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 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 ceb8afdf9a89..afba10684b65 100644
--- a/tools/perf/util/python-ext-sources
+++ b/tools/perf/util/python-ext-sources
@@ -9,6 +9,7 @@ util/python.c
 ../lib/ctype.c
 util/evlist.c
 util/evsel.c
+util/cap.c
 util/cpumap.c
 util/memswap.c
 util/mmap.c
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index a61535cf1bca..4f0da8a03697 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -16,10 +16,12 @@
 #include <string.h>
 #include <errno.h>
 #include <limits.h>
+#include <linux/capability.h>
 #include <linux/kernel.h>
 #include <linux/log2.h>
 #include <linux/time64.h>
 #include <unistd.h>
+#include "cap.h"
 #include "strlist.h"
 #include "string2.h"
 
@@ -443,6 +445,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)
 {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lubashev, Igor]

Thanks for the suggestion!  I'll try to add a test for libcap to the patch series as v2 of the series.  Probably not next week, though (IETF week).

- Igor

> On Wed, July 17, 2019 7:47 PM Arnaldo Carvalho de Melo wrote:
> 
> Em Wed, Jul 17, 2019 at 06:05:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Jul 16, 2019 at 10:46:43AM +0200, Jiri Olsa escreveu:
> > > On Tue, Jul 02, 2019 at 08:10:03PM -0400, Igor Lubashev wrote:
> > > > Add utilities to help checking capabilities of the running process.
> > > > Make perf link with libcap.
> > > >
> > > > Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> > > > ---
> > > >  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
> > >
> > > I wonder we should detect libcap or it's everywhere.. Arnaldo's compile test
> suite might tell
> >
> > I'll add this tentatively and try to build it in my test suite.
> 
> So, not even in my notebook this worked straight away:
> 
>   CC       /tmp/build/perf/util/cap.o
>   CC       /tmp/build/perf/util/config.o
> In file included from util/cap.c:5:
> util/cap.h:6:10: fatal error: sys/capability.h: No such file or directory
>     6 | #include <sys/capability.h>
>       |          ^~~~~~~~~~~~~~~~~~
> compilation terminated.
> mv: cannot stat '/tmp/build/perf/util/.cap.o.tmp': No such file or directory
> 
> 
> I had to first do:
> 
> dnf install libcap-devel
> 
> So we need to have a feature test and fail if that is not installed,
> i.e. libcap becomes a hard req for building perf, which I think is
> reasonable, one more shouldn't hurt, right?
> 
> With all the features enabled:
> 
> [acme@quaco perf]$ ldd ~/bin/perf
> 	linux-vdso.so.1 (0x00007ffe7278a000)
> 	libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8
> (0x00007f7be52f1000)
> 	libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f7be52d7000)
> 	liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f7be52ae000)
> 	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f7be528d000)
> 	librt.so.1 => /lib64/librt.so.1 (0x00007f7be5283000)
> 	libm.so.6 => /lib64/libm.so.6 (0x00007f7be513d000)
> 	libdl.so.2 => /lib64/libdl.so.2 (0x00007f7be5135000)
> 	libcap.so.2 => /lib64/libcap.so.2 (0x00007f7be512e000)
> 	libelf.so.1 => /lib64/libelf.so.1 (0x00007f7be5113000)
> 	libdw.so.1 => /lib64/libdw.so.1 (0x00007f7be50c0000)
> 	libslang.so.2 => /lib64/libslang.so.2 (0x00007f7be4de8000)
> 	libperl.so.5.28 => /lib64/libperl.so.5.28 (0x00007f7be4ac2000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007f7be48fa000)
> 	libpython2.7.so.1.0 => /lib64/libpython2.7.so.1.0 (0x00007f7be4690000)
> 	libz.so.1 => /lib64/libz.so.1 (0x00007f7be4676000)
> 	libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f7be45d1000)
> 	libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f7be45c3000)
> 	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1
> (0x00007f7be456d000)
> 	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f7be4551000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007f7be5331000)
> 	libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f7be453d000)
> 	libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f7be4502000)
> 	libutil.so.1 => /lib64/libutil.so.1 (0x00007f7be44fd000)
> 	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007f7be44ed000)
> 	libpopt.so.0 => /lib64/libpopt.so.0 (0x00007f7be44dd000)
> 	libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f7be44d3000)
> 	libgmodule-2.0.so.0 => /lib64/libgmodule-2.0.so.0 (0x00007f7be44cd000)
> 	libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007f7be43a9000)
> 	libpcre.so.1 => /lib64/libpcre.so.1 (0x00007f7be4335000)
> [acme@quaco perf]$
> 
> ;-)
> 
> So, please check tools/build/feature/ and check how this is done and add
> a test and the warning in tools/perf/Makefile.config so that we get an
> error message stating that libcap-dev or libcap-devel should be
> installed.
> 
> I'll do it if there is any difficulty, just not right now as I'm busy
> and want to get a pull req out of the door.
> 
> - Arnaldo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lubashev, Igor]

On Wed, July 17 at 2019 7:47 PM  Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 17, 2019 at 06:05:51PM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Tue, Jul 16, 2019 at 10:46:43AM +0200, Jiri Olsa escreveu:
> > > On Tue, Jul 02, 2019 at 08:10:03PM -0400, Igor Lubashev wrote:
> > > > Add utilities to help checking capabilities of the running process.
> > > > Make perf link with libcap.
> > > >
> > > > Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
> > > > ---
> > > >  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
> > >
> > > I wonder we should detect libcap or it's everywhere.. Arnaldo's
> > > compile test suite might tell
> >
> > I'll add this tentatively and try to build it in my test suite.
> 
> So, not even in my notebook this worked straight away:
> 
>   CC       /tmp/build/perf/util/cap.o
>   CC       /tmp/build/perf/util/config.o
> In file included from util/cap.c:5:
> util/cap.h:6:10: fatal error: sys/capability.h: No such file or directory
>     6 | #include <sys/capability.h>
>       |          ^~~~~~~~~~~~~~~~~~
> compilation terminated.
> mv: cannot stat '/tmp/build/perf/util/.cap.o.tmp': No such file or directory
> 
> 
> I had to first do:
> 
> dnf install libcap-devel
> 
> So we need to have a feature test and fail if that is not installed, i.e. libcap
> becomes a hard req for building perf, which I think is reasonable, one more
> shouldn't hurt, right?
> 
> With all the features enabled:
> 
> [acme@quaco perf]$ ldd ~/bin/perf
> 	linux-vdso.so.1 (0x00007ffe7278a000)
> 	libunwind-x86_64.so.8 => /lib64/libunwind-x86_64.so.8
> (0x00007f7be52f1000)
> 	libunwind.so.8 => /lib64/libunwind.so.8 (0x00007f7be52d7000)
> 	liblzma.so.5 => /lib64/liblzma.so.5 (0x00007f7be52ae000)
> 	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f7be528d000)
> 	librt.so.1 => /lib64/librt.so.1 (0x00007f7be5283000)
> 	libm.so.6 => /lib64/libm.so.6 (0x00007f7be513d000)
> 	libdl.so.2 => /lib64/libdl.so.2 (0x00007f7be5135000)
> 	libcap.so.2 => /lib64/libcap.so.2 (0x00007f7be512e000)
> 	libelf.so.1 => /lib64/libelf.so.1 (0x00007f7be5113000)
> 	libdw.so.1 => /lib64/libdw.so.1 (0x00007f7be50c0000)
> 	libslang.so.2 => /lib64/libslang.so.2 (0x00007f7be4de8000)
> 	libperl.so.5.28 => /lib64/libperl.so.5.28 (0x00007f7be4ac2000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007f7be48fa000)
> 	libpython2.7.so.1.0 => /lib64/libpython2.7.so.1.0
> (0x00007f7be4690000)
> 	libz.so.1 => /lib64/libz.so.1 (0x00007f7be4676000)
> 	libzstd.so.1 => /lib64/libzstd.so.1 (0x00007f7be45d1000)
> 	libnuma.so.1 => /lib64/libnuma.so.1 (0x00007f7be45c3000)
> 	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1
> (0x00007f7be456d000)
> 	libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f7be4551000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007f7be5331000)
> 	libbz2.so.1 => /lib64/libbz2.so.1 (0x00007f7be453d000)
> 	libcrypt.so.2 => /lib64/libcrypt.so.2 (0x00007f7be4502000)
> 	libutil.so.1 => /lib64/libutil.so.1 (0x00007f7be44fd000)
> 	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1
> (0x00007f7be44ed000)
> 	libpopt.so.0 => /lib64/libpopt.so.0 (0x00007f7be44dd000)
> 	libuuid.so.1 => /lib64/libuuid.so.1 (0x00007f7be44d3000)
> 	libgmodule-2.0.so.0 => /lib64/libgmodule-2.0.so.0
> (0x00007f7be44cd000)
> 	libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x00007f7be43a9000)
> 	libpcre.so.1 => /lib64/libpcre.so.1 (0x00007f7be4335000)
> [acme@quaco perf]$
> 
> ;-)
> 
> So, please check tools/build/feature/ and check how this is done and add a
> test and the warning in tools/perf/Makefile.config so that we get an error
> message stating that libcap-dev or libcap-devel should be installed.

I have just posted v2 of the series (https://lkml.kernel.org/lkml/cover.1565146171.git.ilubashe@akamai.com).

Instead of making libcap is "hard req", I made it as "soft" one. We can still build a useful tool w/o libcap. It will just have to assume that perf is running with no capabilities, since we cannot query them.

Many thanks for the pointers on how to go about build feature checking.

- Igor

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Igor Lubashev]

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

Signed-off-by: Igor Lubashev <ilubashe@akamai.com>
---
 tools/perf/arch/arm/util/cs-etm.c    | 3 ++-
 tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
 tools/perf/arch/x86/util/intel-bts.c | 3 ++-
 tools/perf/arch/x86/util/intel-pt.c  | 2 +-
 tools/perf/util/evsel.c              | 2 +-
 5 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c
index 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 <time.h>
 
 #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 <linux/log2.h>
 
 #include "../../util/cpumap.h"
+#include "../../util/event.h"
 #include "../../util/evsel.h"
 #include "../../util/evlist.h"
 #include "../../util/session.h"
@@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr,
 	struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu;
 	struct 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jiri Olsa]

On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote:
> The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> perf_event_paranoid check. Make perf do the same.

I see another geteuid check in __cmd_ftrace,
perhaps we should cover this one as well

jirka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lubashev, Igor]

I could add another patch to the series for that.  Any suggestion for what capability to check for here?

(There is always an alternative to not check for anything and let the kernel refuse to perform actions that the user does not have permissions to perform.)

- Igor

-----Original Message-----
From: Jiri Olsa <jolsa@redhat.com> 
Sent: Tuesday, July 16, 2019 4:48 AM
Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks

On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote:
> The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> perf_event_paranoid check. Make perf do the same.

I see another geteuid check in __cmd_ftrace,
perhaps we should cover this one as well

jirka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jiri Olsa]

On Tue, Jul 16, 2019 at 05:01:26PM +0000, Lubashev, Igor wrote:
> I could add another patch to the series for that.  Any suggestion for what capability to check for here?

it's:

	if (geteuid() != 0) {
		pr_err("ftrace only works for root!\n");
		return -1
	}

so I think check for CAP_SYS_ADMIN should be fine in here

jirka

> 
> (There is always an alternative to not check for anything and let the kernel refuse to perform actions that the user does not have permissions to perform.)
> 
> - Igor
> 
> -----Original Message-----
> From: Jiri Olsa <jolsa@redhat.com> 
> Sent: Tuesday, July 16, 2019 4:48 AM
> Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> 
> On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote:
> > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> > perf_event_paranoid check. Make perf do the same.
> 
> I see another geteuid check in __cmd_ftrace,
> perhaps we should cover this one as well
> 
> jirka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Lubashev, Igor]

> On Wednesday, July 17, 2019 3:10 AM Jiri Olsa wrote:
> On Tue, Jul 16, 2019 at 05:01:26PM +0000, Lubashev, Igor wrote:
> > I could add another patch to the series for that.  Any suggestion for what
> capability to check for here?
> 
> it's:
> 
> 	if (geteuid() != 0) {
> 		pr_err("ftrace only works for root!\n");
> 		return -1
> 	}
> 
> so I think check for CAP_SYS_ADMIN should be fine in here

Thanks.  Added the [PATCH 4/3] to this series (https://lore.kernel.org/lkml/1563387359-27694-1-git-send-email-ilubashe@akamai.com/).
Let me know if you'd rather I reroll a V2 of this series.

- Igor


> 
> jirka
> 
> >
> > (There is always an alternative to not check for anything and let the kernel
> refuse to perform actions that the user does not have permissions to perform.)
> >
> > - Igor
> >
> > -----Original Message-----
> > From: Jiri Olsa <jolsa@redhat.com>
> > Sent: Tuesday, July 16, 2019 4:48 AM
> > Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid
> checks
> >
> > On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote:
> > > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override
> > > perf_event_paranoid check. Make perf do the same.
> >
> > I see another geteuid check in __cmd_ftrace,
> > perhaps we should cover this one as well
> >
> > jirka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[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 <ilubashe@akamai.com>
---
 tools/perf/util/symbol.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5cbad55cd99d..fd68dae3f58e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <linux/capability.h>
 #include <linux/kernel.h>
 #include <linux/mman.h>
 #include <linux/time64.h>
@@ -15,8 +16,10 @@
 #include <inttypes.h>
 #include "annotate.h"
 #include "build-id.h"
+#include "cap.h"
 #include "util.h"
 #include "debug.h"
+#include "event.h"
 #include "machine.h"
 #include "map.h"
 #include "symbol.h"
@@ -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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Alexey Budankov]

On 03.07.2019 3:10, Igor Lubashev wrote:
> 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.

Acked-by: Alexey Budankov <alexey.budankov@linux.intel.com>

Valuable contribution, thanks! And I see the continuation of the effort started 
in this patch set. Some dedicated CAP_SYS_PERFMON capability could be introduced 
and used for performance monitoring related security checks, as in the kernel as 
in the user mode, because CAP_SYS_ADMIN grants much wider credentials that are 
required, at least for Perf related monitoring and, yet more, CAP_SYS_ADMIN could 
be unloaded addressing the concerns here [1]:

 CAP_SYS_ADMIN
       	   Note: this capability is overloaded; see Notes to kernel developers, below.
 ...
 Notes to kernel developers:
	   When adding a new kernel feature that should be governed by a
	   capability, consider the following points.
	   *  The goal of capabilities is divide the power of superuser into
	       pieces, such that if a program that has one or more capabilities
	       is compromised, its power to do damage to the system would be less
	       than the same program running with root privilege.
	   *  You have the choice of either creating a new capability for your
	       new feature, or associating the feature with one of the existing
	       capabilities.  In order to keep the set of capabilities to a
	       manageable size, the latter option is preferable, unless there are
	       compelling reasons to take the former option.  (There is also a
	       technical limit: the size of capability sets is currently limited
	       to 64 bits.)
	       . . .
	    * Don't choose CAP_SYS_ADMIN if you can possibly avoid it!  A vast
	       proportion of existing capability checks are associated with this
	       capability (see the partial list above).  It can plausibly be
	       called "the new root", since on the one hand, it confers a wide
	       range of powers, and on the other hand, its broad scope means that
	       this is the capability that is required by many privileged
	       programs.  Don't make the problem worse.  The only new features
	       that should be associated with CAP_SYS_ADMIN are ones that closely
	       match existing uses in that silo.
	    * If you have determined that it really is necessary to create a new
	       capability for your feature, don't make or name it as a "single-
	       use" capability.  Thus, for example, the addition of the highly
	       specific CAP_SYS_PACCT was probably a mistake.  Instead, try to
	       identify and name your new capability as a broader silo into which
           other related future use cases might fit.”

Regards,
Alexey

[1] http://man7.org/linux/man-pages/man7/capabilities.7.html

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel