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

[permalink] [raw]

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

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

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 173f3378aaa0..046271103499 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,6 +4,7 @@
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
+#include <linux/capability.h>
#include <linux/kernel.h>
#include <linux/mman.h>
#include <linux/time64.h>
@@ -15,8 +16,10 @@
#include <inttypes.h>
#include "annotate.h"
#include "build-id.h"
+#include "cap.h"
#include "util.h"
#include "debug.h"
+#include "event.h"
#include "machine.h"
#include "map.h"
#include "symbol.h"
@@ -890,7 +893,11 @@ bool symbol__restricted_filename(const char *filename,
{
bool restricted = false;

- if (symbol_conf.kptr_restrict) {
+ /* Per kernel/kallsyms.c:
+ * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+ */
+ if (symbol_conf.kptr_restrict ||
+ (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))) {
char *r = realpath(filename, NULL);

if (r != NULL) {
@@ -2190,9 +2197,9 @@ static bool symbol__read_kptr_restrict(void)
char line[8];

if (fgets(line, sizeof(line), fp) != NULL)
- value = ((geteuid() != 0) || (getuid() != 0)) ?
- (atoi(line) != 0) :
- (atoi(line) == 2);
+ value = perf_cap__capable(CAP_SYSLOG) ?
+ (atoi(line) >= 2) :
+ (atoi(line) != 0);

fclose(fp);
}
--
2.7.4

2019-08-12 09:14:55

by Jiri Olsa

[permalink] [raw]

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

On Wed, Aug 07, 2019 at 10:44:13AM -0400, Igor Lubashev wrote:
> Series v1: https://lkml.kernel.org/lkml/[email protected]
>
>
> Kernel is using capabilities instead of uid and euid to restrict access to
> kernel pointers and tracing facilities. This patch series updates the perf to
> better match the security model used by the kernel.
>
> This series enables instructions in Documentation/admin-guide/perf-security.rst
> to actually work, even when kernel.perf_event_paranoid=2 and
> kernel.kptr_restrict=1.
>
> The series consists of four patches:
>
> 01: perf: Add capability-related utilities
> Add utility functions to check capabilities and perf_event_paranoid checks,
> if libcap-dev[el] is available. (Otherwise, assume no capabilities.)
>
> 02: perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
> perf_event_paranoid level is verified.
>
> 03: perf: Use CAP_SYSLOG with kptr_restrict checks
> Replace the use of uid and euid with a check for CAP_SYSLOG when
> kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
> Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).
>
> 04: perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
> Replace the use of euid==0 with a check for CAP_SYS_ADMIN before mounting
> debugfs for ftrace.
>
> I tested this by following Documentation/admin-guide/perf-security.rst
> guidelines and setting sysctls:
>
> kernel.perf_event_paranoid=2
> kernel.kptr_restrict=1
>
> As an unprivileged user who is in perf_users group (setup via instructions
> above), I executed:
> perf record -a -- sleep 1
>
> Without the patch, perf record did not capture any kernel functions.
> With the patch, perf included all kernel functions.
>
>
> Changelog:
> v3: * Fix arm64 compilation (thanks, Alexey and Jiri)

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

> v2: * Added a build feature check for libcap-dev[el] as suggested by Arnaldo
>
>
> Igor Lubashev (4):
> perf: Add capability-related utilities
> perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> perf: Use CAP_SYSLOG with kptr_restrict checks
> perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace
>
> tools/build/Makefile.feature | 2 ++
> tools/build/feature/Makefile | 4 ++++
> tools/build/feature/test-libcap.c | 20 ++++++++++++++++++++
> tools/perf/Makefile.config | 11 +++++++++++
> tools/perf/Makefile.perf | 2 ++
> tools/perf/arch/arm/util/cs-etm.c | 3 ++-
> tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
> tools/perf/arch/x86/util/intel-bts.c | 3 ++-
> tools/perf/arch/x86/util/intel-pt.c | 2 +-
> tools/perf/builtin-ftrace.c | 4 +++-
> tools/perf/util/Build | 2 ++
> tools/perf/util/cap.c | 29 +++++++++++++++++++++++++++++
> tools/perf/util/cap.h | 24 ++++++++++++++++++++++++
> tools/perf/util/event.h | 1 +
> tools/perf/util/evsel.c | 2 +-
> tools/perf/util/python-ext-sources | 1 +
> tools/perf/util/symbol.c | 15 +++++++++++----
> tools/perf/util/util.c | 9 +++++++++
> 18 files changed, 127 insertions(+), 10 deletions(-)
> create mode 100644 tools/build/feature/test-libcap.c
> create mode 100644 tools/perf/util/cap.c
> create mode 100644 tools/perf/util/cap.h
>
> --
> 2.7.4
>

2019-08-14 18:07:41

[permalink] [raw]

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

On Wed, 7 Aug 2019 at 08:44, Igor Lubashev <[email protected]> wrote:
>
> Kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0 when
> checking kptr_restrict. Make perf do the same.
>
> Also, the kernel is a more restrictive than "no restrictions" in case of
> kptr_restrict==0, so add the same logic to perf.
>
> Signed-off-by: Igor Lubashev <[email protected]>
> ---
> tools/perf/util/symbol.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 173f3378aaa0..046271103499 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -4,6 +4,7 @@
> #include <stdlib.h>
> #include <stdio.h>
> #include <string.h>
> +#include <linux/capability.h>
> #include <linux/kernel.h>
> #include <linux/mman.h>
> #include <linux/time64.h>
> @@ -15,8 +16,10 @@
> #include <inttypes.h>
> #include "annotate.h"
> #include "build-id.h"
> +#include "cap.h"
> #include "util.h"
> #include "debug.h"
> +#include "event.h"
> #include "machine.h"
> #include "map.h"
> #include "symbol.h"
> @@ -890,7 +893,11 @@ bool symbol__restricted_filename(const char *filename,
> {
> bool restricted = false;
>
> - if (symbol_conf.kptr_restrict) {
> + /* Per kernel/kallsyms.c:
> + * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
> + */
> + if (symbol_conf.kptr_restrict ||
> + (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))) {
> char *r = realpath(filename, NULL);
>

# echo 0 > /proc/sys/kernel/kptr_restrict
# ./tools/perf/perf record -e instructions:k uname
perf: Segmentation fault
Obtained 10 stack frames.
./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
/lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
Segmentation fault

I can reproduce this on both x86 and ARM64.

> if (r != NULL) {
> @@ -2190,9 +2197,9 @@ static bool symbol__read_kptr_restrict(void)
> char line[8];
>
> if (fgets(line, sizeof(line), fp) != NULL)
> - value = ((geteuid() != 0) || (getuid() != 0)) ?
> - (atoi(line) != 0) :
> - (atoi(line) == 2);
> + value = perf_cap__capable(CAP_SYSLOG) ?
> + (atoi(line) >= 2) :
> + (atoi(line) != 0);
>
> fclose(fp);
> }
> --
> 2.7.4
>

2019-08-14 18:50:28

[permalink] [raw]

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

Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> On Wed, 7 Aug 2019 at 08:44, Igor Lubashev <[email protected]> wrote:
> >
> > Kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0 when
> > checking kptr_restrict. Make perf do the same.
> >
> > Also, the kernel is a more restrictive than "no restrictions" in case of
> > kptr_restrict==0, so add the same logic to perf.
> >
> > Signed-off-by: Igor Lubashev <[email protected]>
> > ---
> > tools/perf/util/symbol.c | 15 +++++++++++----
> > 1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 173f3378aaa0..046271103499 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -4,6 +4,7 @@
> > #include <stdlib.h>
> > #include <stdio.h>
> > #include <string.h>
> > +#include <linux/capability.h>
> > #include <linux/kernel.h>
> > #include <linux/mman.h>
> > #include <linux/time64.h>
> > @@ -15,8 +16,10 @@
> > #include <inttypes.h>
> > #include "annotate.h"
> > #include "build-id.h"
> > +#include "cap.h"
> > #include "util.h"
> > #include "debug.h"
> > +#include "event.h"
> > #include "machine.h"
> > #include "map.h"
> > #include "symbol.h"
> > @@ -890,7 +893,11 @@ bool symbol__restricted_filename(const char *filename,
> > {
> > bool restricted = false;
> >
> > - if (symbol_conf.kptr_restrict) {
> > + /* Per kernel/kallsyms.c:
> > + * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
> > + */
> > + if (symbol_conf.kptr_restrict ||
> > + (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))) {
> > char *r = realpath(filename, NULL);
> >
>
> # echo 0 > /proc/sys/kernel/kptr_restrict
> # ./tools/perf/perf record -e instructions:k uname
> perf: Segmentation fault
> Obtained 10 stack frames.
> ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
> ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
> ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
> Segmentation fault
>
> I can reproduce this on both x86 and ARM64.

I don't see this with these two csets removed:

7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks

Which were the ones I guessed were related to the problem you reported,
so they are out of my ongoing perf/core pull request to Ingo/Thomas, now
trying with these applied and your instructions...

- Arnaldo

2019-08-14 19:10:27

[permalink] [raw]

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

Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > # echo 0 > /proc/sys/kernel/kptr_restrict
> > # ./tools/perf/perf record -e instructions:k uname
> > perf: Segmentation fault
> > Obtained 10 stack frames.
> > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7) [0x55af9e590337]
> > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7) [0x7fd31ef99b97]
> > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a]
> > Segmentation fault
> >
> > I can reproduce this on both x86 and ARM64.
>
> I don't see this with these two csets removed:
>
> 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
>
> Which were the ones I guessed were related to the problem you reported,
> so they are out of my ongoing perf/core pull request to Ingo/Thomas, now
> trying with these applied and your instructions...

Can't repro:

[root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
0
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]# echo 1 > /proc/sys/kernel/kptr_restrict
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]# echo 0 > /proc/sys/kernel/kptr_restrict
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
[root@quaco ~]#

[acme@quaco perf]$ git log --oneline --author Lubashev tools/
7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap, acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
c766f3df635d perf ftrace: Use CAP_SYS_ADMIN instead of euid==0
c22e150e3afa perf tools: Add helpers to use capabilities if present
74d5f3d06f70 tools build: Add capability-related feature detection
perf version 5.3.rc4.g7ff5b5911144
[acme@quaco perf]$

- Arnaldo

2019-08-14 20:39:41

[permalink] [raw]

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

> On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> escreveu:
> > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > -e instructions:k uname
> > > perf: Segmentation fault
> > > Obtained 10 stack frames.
> > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > [0x55af9e590337]
> > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > [0x7fd31ef99b97]
> > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > >
> > > I can reproduce this on both x86 and ARM64.
> >
> > I don't see this with these two csets removed:
> >
> > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > checks
> >
> > Which were the ones I guessed were related to the problem you
> > reported, so they are out of my ongoing perf/core pull request to
> > Ingo/Thomas, now trying with these applied and your instructions...
>
> Can't repro:
>
> [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> 0
> [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> [root@quaco ~]#
>
> [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> present
> 74d5f3d06f70 tools build: Add capability-related feature detection perf
> version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$

I got an ARM64 cloud VM, but I cannot reproduce.
# cat /proc/sys/kernel/kptr_restrict
0

Perf trace works fine (does not die):
# ./perf trace -a

Here is my setup:
Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
Branch: tmp.perf/cap
Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
lsb_release -a: Ubuntu 18.04.3 LTS

Auto-detecting system features:
... dwarf: [ on ]
... dwarf_getlocations: [ on ]
... glibc: [ on ]
... gtk2: [ on ]
... libaudit: [ on ]
... libbfd: [ on ]
... libcap: [ on ]
... libelf: [ on ]
... libnuma: [ on ]
... numa_num_possible_cpus: [ on ]
... libperl: [ on ]
... libpython: [ on ]
... libcrypto: [ on ]
... libunwind: [ on ]
... libdw-dwarf-unwind: [ on ]
... zlib: [ on ]
... lzma: [ on ]
... get_cpuid: [ OFF ]
... bpf: [ on ]
... libaio: [ on ]
... libzstd: [ on ]
... disassembler-four-args: [ on ]

I also could not reproduce on x86:
lsb_release -a: Ubuntu 18.04.1 LTS
gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
uname -r: 4.4.0-154-generic

2019-08-15 16:39:30

[permalink] [raw]

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

On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]> wrote:
>
> > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > -e instructions:k uname
> > > > perf: Segmentation fault
> > > > Obtained 10 stack frames.
> > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > [0x55af9e590337]
> > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > [0x7fd31ef99b97]
> > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > >
> > > > I can reproduce this on both x86 and ARM64.
> > >
> > > I don't see this with these two csets removed:
> > >
> > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > checks
> > >
> > > Which were the ones I guessed were related to the problem you
> > > reported, so they are out of my ongoing perf/core pull request to
> > > Ingo/Thomas, now trying with these applied and your instructions...
> >
> > Can't repro:
> >
> > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > [root@quaco ~]#
> >
> > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > present
> > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
>
> I got an ARM64 cloud VM, but I cannot reproduce.
> # cat /proc/sys/kernel/kptr_restrict
> 0
>
> Perf trace works fine (does not die):
> # ./perf trace -a
>
> Here is my setup:
> Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> Branch: tmp.perf/cap
> Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> lsb_release -a: Ubuntu 18.04.3 LTS
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> ... gtk2: [ on ]
> ... libaudit: [ on ]
> ... libbfd: [ on ]
> ... libcap: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ on ]
> ... libdw-dwarf-unwind: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ OFF ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
> ... disassembler-four-args: [ on ]

Thank you for posting your configuration. I will see where things are
different with mine.

>
> I also could not reproduce on x86:
> lsb_release -a: Ubuntu 18.04.1 LTS
> gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> uname -r: 4.4.0-154-generic

2019-08-15 23:02:57

[permalink] [raw]

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

On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]> wrote:
>
> > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > escreveu:
> > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > -e instructions:k uname
> > > > perf: Segmentation fault
> > > > Obtained 10 stack frames.
> > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > [0x55af9e590337]
> > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > [0x7fd31ef99b97]
> > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > >
> > > > I can reproduce this on both x86 and ARM64.
> > >
> > > I don't see this with these two csets removed:
> > >
> > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > checks
> > >
> > > Which were the ones I guessed were related to the problem you
> > > reported, so they are out of my ongoing perf/core pull request to
> > > Ingo/Thomas, now trying with these applied and your instructions...
> >
> > Can't repro:
> >
> > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > [root@quaco ~]#
> >
> > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > present
> > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
>
> I got an ARM64 cloud VM, but I cannot reproduce.
> # cat /proc/sys/kernel/kptr_restrict
> 0
>
> Perf trace works fine (does not die):
> # ./perf trace -a
>
> Here is my setup:
> Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> Branch: tmp.perf/cap
> Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> lsb_release -a: Ubuntu 18.04.3 LTS
>
> Auto-detecting system features:
> ... dwarf: [ on ]
> ... dwarf_getlocations: [ on ]
> ... glibc: [ on ]
> ... gtk2: [ on ]
> ... libaudit: [ on ]
> ... libbfd: [ on ]
> ... libcap: [ on ]
> ... libelf: [ on ]
> ... libnuma: [ on ]
> ... numa_num_possible_cpus: [ on ]
> ... libperl: [ on ]
> ... libpython: [ on ]
> ... libcrypto: [ on ]
> ... libunwind: [ on ]
> ... libdw-dwarf-unwind: [ on ]
> ... zlib: [ on ]
> ... lzma: [ on ]
> ... get_cpuid: [ OFF ]
> ... bpf: [ on ]
> ... libaio: [ on ]
> ... libzstd: [ on ]
> ... disassembler-four-args: [ on ]
>
> I also could not reproduce on x86:
> lsb_release -a: Ubuntu 18.04.1 LTS
> gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> uname -r: 4.4.0-154-generic

I isolated the problem to libcap-dev - if it is not installed a
segmentation fault will occur. Since this set is about using
capabilities it is obvious that not having it on a system should fail
a trace session, but it should not crash it.

If libcap-dev is not installed function symbol__restricted_filename()
will return true, which in turn will prevent symbol_name to be set in
machine__get_running_kernel_start(). That prevents function
map__set_kallsyms_ref_reloc_sym() from being called in
machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
fault.

I am not sure how this can be fixed. I counted a total of 19
instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
care to check if kmap->ref_reloc_sym is valid before proceeding. As
such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
guaranteed to be valid. If I am correct then all we need is to check
for a valid pointer in _perf_event__synthesize_kernel_mmap().
Otherwise it will be a little harder.

Mathieu

2019-08-15 23:43:58

[permalink] [raw]

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

Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]> wrote:
> >
> > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > escreveu:
> > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > > -e instructions:k uname
> > > > > perf: Segmentation fault
> > > > > Obtained 10 stack frames.
> > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > [0x55af9e590337]
> > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > [0x7fd31ef99b97]
> > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > > >
> > > > > I can reproduce this on both x86 and ARM64.
> > > >
> > > > I don't see this with these two csets removed:
> > > >
> > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > > checks
> > > >
> > > > Which were the ones I guessed were related to the problem you
> > > > reported, so they are out of my ongoing perf/core pull request to
> > > > Ingo/Thomas, now trying with these applied and your instructions...
> > >
> > > Can't repro:
> > >
> > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > 0
> > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > > [root@quaco ~]#
> > >
> > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > > present
> > > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> >
> > I got an ARM64 cloud VM, but I cannot reproduce.
> > # cat /proc/sys/kernel/kptr_restrict
> > 0
> >
> > Perf trace works fine (does not die):
> > # ./perf trace -a
> >
> > Here is my setup:
> > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > Branch: tmp.perf/cap
> > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> > lsb_release -a: Ubuntu 18.04.3 LTS
> >
> > Auto-detecting system features:
> > ... dwarf: [ on ]
> > ... dwarf_getlocations: [ on ]
> > ... glibc: [ on ]
> > ... gtk2: [ on ]
> > ... libaudit: [ on ]
> > ... libbfd: [ on ]
> > ... libcap: [ on ]
> > ... libelf: [ on ]
> > ... libnuma: [ on ]
> > ... numa_num_possible_cpus: [ on ]
> > ... libperl: [ on ]
> > ... libpython: [ on ]
> > ... libcrypto: [ on ]
> > ... libunwind: [ on ]
> > ... libdw-dwarf-unwind: [ on ]
> > ... zlib: [ on ]
> > ... lzma: [ on ]
> > ... get_cpuid: [ OFF ]
> > ... bpf: [ on ]
> > ... libaio: [ on ]
> > ... libzstd: [ on ]
> > ... disassembler-four-args: [ on ]
> >
> > I also could not reproduce on x86:
> > lsb_release -a: Ubuntu 18.04.1 LTS
> > gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> > uname -r: 4.4.0-154-generic
>
> I isolated the problem to libcap-dev - if it is not installed a
> segmentation fault will occur. Since this set is about using
> capabilities it is obvious that not having it on a system should fail
> a trace session, but it should not crash it.

It shouldn't crash on x86_64:

root@quaco ~]# sysctl kernel.kptr_restrict
kernel.kptr_restrict = 0
[root@quaco ~]# perf record -e instructions:k uname
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.023 MB perf.data (5 samples) ]
[root@quaco ~]# ldd ~/bin/perf | grep libcap
[root@quaco ~]# perf -v
perf version 5.3.rc4.g329ca330bf8b
[root@quaco ~]#
[acme@quaco perf]$ git log --oneline -4
329ca330bf8b (HEAD -> perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
f7b9999387af perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf report: Add --switch-on/--switch-off events
2f53ae347f59 perf top: Add --switch-on/--switch-off events
[acme@quaco perf]$

> If libcap-dev is not installed function symbol__restricted_filename()
> will return true, which in turn will prevent symbol_name to be set in
> machine__get_running_kernel_start(). That prevents function
> map__set_kallsyms_ref_reloc_sym() from being called in
> machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
> fault.

Can you please try with my perf/cap branch? Perhaps you're using Igor's
original set of patches? I made changes to the fallback, he was making
it return false while I made it fallback to geteuid() == 0, as was
before his patches.

- Arnaldo

> I am not sure how this can be fixed. I counted a total of 19
> instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> care to check if kmap->ref_reloc_sym is valid before proceeding. As
> such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> guaranteed to be valid. If I am correct then all we need is to check
> for a valid pointer in _perf_event__synthesize_kernel_mmap().
> Otherwise it will be a little harder.
>
> Mathieu

--

- Arnaldo

2019-08-15 23:49:54

[permalink] [raw]

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

On Thu, August 15, 2019 at 4:17 PM Mathieu Poirier <[email protected]> wrote:
> On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]>
> wrote:
> >
> > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > escreveu:
> > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf
> > > > > record -e instructions:k uname
> > > > > perf: Segmentation fault
> > > > > Obtained 10 stack frames.
> > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > [0x55af9e590337]
> > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > [0x7fd31ef99b97]
> > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > fault
> > > > >
> > > > > I can reproduce this on both x86 and ARM64.
> > > >
> > > > I don't see this with these two csets removed:
> > > >
> > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > perf_event_paranoid checks
> > > >
> > > > Which were the ones I guessed were related to the problem you
> > > > reported, so they are out of my ongoing perf/core pull request to
> > > > Ingo/Thomas, now trying with these applied and your instructions...

SNIP

> I isolated the problem to libcap-dev - if it is not installed a segmentation fault
> will occur. Since this set is about using capabilities it is obvious that not
> having it on a system should fail a trace session, but it should not crash it.
>
> If libcap-dev is not installed function symbol__restricted_filename() will
> return true, which in turn will prevent symbol_name to be set in
> machine__get_running_kernel_start(). That prevents function
> map__set_kallsyms_ref_reloc_sym() from being called in
> machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> NULL in _perf_event__synthesize_kernel_mmap() and a segmentation fault.

Thank you, great find.

> I am not sure how this can be fixed. I counted a total of 19 instances where
> kmap->ref_reloc_sym->XYZ is called, only 2 of wich care to check if kmap-
> >ref_reloc_sym is valid before proceeding. As such I must hope that in the
> 17 other cases, kmap->ref_reloc_sym is guaranteed to be valid. If I am
> correct then all we need is to check for a valid pointer in
> _perf_event__synthesize_kernel_mmap().
> Otherwise it will be a little harder.

I also see 19 instances in 5 files, but by my inspection all cases but one are ok (the code checks for NULL earlier in the function or in a helper function).

The not ok case is __perf_event__synthesize_kermel_mmap(), which simply checks symbol_conf.kptr_restrict.

==== Option 1 =====
Fix __perf_event__synthesize_kermel_mmap(). This probably should be done regardless.

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index f440fdc3e953..b84f5f3c9651 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -913,11 +913,13 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
int err;
union perf_event *event;

- if (symbol_conf.kptr_restrict)
- return -1;
if (map == NULL)
return -1;

+ kmap = map__kmap(map);
+ if (!kmap->ref_reloc_sym)
+ return -1;
+
/*
* We should get this from /sys/kernel/sections/.text, but till that is
* available use this, and after it is use this as a fallback for older
@@ -940,7 +942,6 @@ static int __perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
}

- kmap = map__kmap(map);
size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
"%s%s", machine->mmap_name, kmap->ref_reloc_sym->name) + 1;
size = PERF_ALIGN(size, sizeof(u64));
--

==== Option 2 =====
Move the new perf_event_paranoid() check from symbol__restricted_filename() to symbol__read_kptr_restrict().
Other than the use above, kptr_restrict is only used for printing warnings.

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7409d2facd5b..035f2e75728c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -898,11 +898,7 @@ bool symbol__restricted_filename(const char *filename,
{
bool restricted = false;

- /* Per kernel/kallsyms.c:
- * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
- */
- if (symbol_conf.kptr_restrict ||
- (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))) {
+ if (symbol_conf.kptr_restrict) {
char *r = realpath(filename, NULL);

if (r != NULL) {
@@ -2209,6 +2205,12 @@ static bool symbol__read_kptr_restrict(void)
fclose(fp);
}

+ /* Per kernel/kallsyms.c:
+ * we also restrict when perf_event_paranoid > 1 w/o CAP_SYSLOG
+ */
+ if (perf_event_paranoid() > 1 && !perf_cap__capable(CAP_SYSLOG))
+ value = true;
+
return value;
}

--------- And then update the warnings. -----------

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

if (symbol_conf.kptr_restrict) {
pr_warning("Kernel address maps (/proc/{kallsyms,modules}) are restricted.\n\n"
- "Check /proc/sys/kernel/kptr_restrict.\n\n"
+ "Check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.\n\n"
"Kernel samples will not be resolved.\n");
machine->kptr_restrict_warned = true;
return NULL;

- Igor

2019-08-19 16:53:10

[permalink] [raw]

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

On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]> wrote:
> > >
> > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de Melo
> > > > escreveu:
> > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict # ./tools/perf/perf record
> > > > > > -e instructions:k uname
> > > > > > perf: Segmentation fault
> > > > > > Obtained 10 stack frames.
> > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44) [0x55af9e5da5d4]
> > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > [0x55af9e590337]
> > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > [0x7fd31ef99b97]
> > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation fault
> > > > > >
> > > > > > I can reproduce this on both x86 and ARM64.
> > > > >
> > > > > I don't see this with these two csets removed:
> > > > >
> > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> > > > > d7604b66102e perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid
> > > > > checks
> > > > >
> > > > > Which were the ones I guessed were related to the problem you
> > > > > reported, so they are out of my ongoing perf/core pull request to
> > > > > Ingo/Thomas, now trying with these applied and your instructions...
> > > >
> > > > Can't repro:
> > > >
> > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > 0
> > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf record:
> > > > Woken up 1 times to write data ] [ perf record: Captured and wrote 0.024 MB
> > > > perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e instructions:k
> > > > uname Linux [ perf record: Woken up 1 times to write data ] [ perf record:
> > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo
> > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > instructions:k uname Linux [ perf record: Woken up 1 times to write data ] [
> > > > perf record: Captured and wrote 0.024 MB perf.data (1 samples) ]
> > > > [root@quaco ~]#
> > > >
> > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > perf_event_paranoid checks c766f3df635d perf ftrace: Use CAP_SYS_ADMIN
> > > > instead of euid==0 c22e150e3afa perf tools: Add helpers to use capabilities if
> > > > present
> > > > 74d5f3d06f70 tools build: Add capability-related feature detection perf
> > > > version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > >
> > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > # cat /proc/sys/kernel/kptr_restrict
> > > 0
> > >
> > > Perf trace works fine (does not die):
> > > # ./perf trace -a
> > >
> > > Here is my setup:
> > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > Branch: tmp.perf/cap
> > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict checks"
> > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux
> > > lsb_release -a: Ubuntu 18.04.3 LTS
> > >
> > > Auto-detecting system features:
> > > ... dwarf: [ on ]
> > > ... dwarf_getlocations: [ on ]
> > > ... glibc: [ on ]
> > > ... gtk2: [ on ]
> > > ... libaudit: [ on ]
> > > ... libbfd: [ on ]
> > > ... libcap: [ on ]
> > > ... libelf: [ on ]
> > > ... libnuma: [ on ]
> > > ... numa_num_possible_cpus: [ on ]
> > > ... libperl: [ on ]
> > > ... libpython: [ on ]
> > > ... libcrypto: [ on ]
> > > ... libunwind: [ on ]
> > > ... libdw-dwarf-unwind: [ on ]
> > > ... zlib: [ on ]
> > > ... lzma: [ on ]
> > > ... get_cpuid: [ OFF ]
> > > ... bpf: [ on ]
> > > ... libaio: [ on ]
> > > ... libzstd: [ on ]
> > > ... disassembler-four-args: [ on ]
> > >
> > > I also could not reproduce on x86:
> > > lsb_release -a: Ubuntu 18.04.1 LTS
> > > gcc --version: gcc (Ubuntu 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0
> > > uname -r: 4.4.0-154-generic
> >
> > I isolated the problem to libcap-dev - if it is not installed a
> > segmentation fault will occur. Since this set is about using
> > capabilities it is obvious that not having it on a system should fail
> > a trace session, but it should not crash it.
>
> It shouldn't crash on x86_64:
>
> root@quaco ~]# sysctl kernel.kptr_restrict
> kernel.kptr_restrict = 0
> [root@quaco ~]# perf record -e instructions:k uname
> Linux
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.023 MB perf.data (5 samples) ]
> [root@quaco ~]# ldd ~/bin/perf | grep libcap
> [root@quaco ~]# perf -v
> perf version 5.3.rc4.g329ca330bf8b
> [root@quaco ~]#
> [acme@quaco perf]$ git log --oneline -4
> 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> f7b9999387af perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf report: Add --switch-on/--switch-off events
> 2f53ae347f59 perf top: Add --switch-on/--switch-off events
> [acme@quaco perf]$
>
> > If libcap-dev is not installed function symbol__restricted_filename()
> > will return true, which in turn will prevent symbol_name to be set in
> > machine__get_running_kernel_start(). That prevents function
> > map__set_kallsyms_ref_reloc_sym() from being called in
> > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym being
> > NULL in _perf_event__synthesize_kernel_mmap() and a segmentation
> > fault.
>
> Can you please try with my perf/cap branch? Perhaps you're using Igor's
> original set of patches? I made changes to the fallback, he was making
> it return false while I made it fallback to geteuid() == 0, as was
> before his patches.

Things are working properly on your perf/cap branch. I tested with on
both x86 and ARM.

Mathieu

>
> - Arnaldo
>
> > I am not sure how this can be fixed. I counted a total of 19
> > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > guaranteed to be valid. If I am correct then all we need is to check
> > for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > Otherwise it will be a little harder.
> >
> > Mathieu
>
> --
>
> - Arnaldo

2019-08-19 22:23:54

[permalink] [raw]

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

On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <[email protected]> wrote:
> On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]>
> wrote:
> > > >
> > > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> > > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de
> > > > > Melo
> > > > > escreveu:
> > > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier
> escreveu:
> > > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict #
> > > > > > > ./tools/perf/perf record -e instructions:k uname
> > > > > > > perf: Segmentation fault
> > > > > > > Obtained 10 stack frames.
> > > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44)
> > > > > > > [0x55af9e5da5d4]
> > > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > > [0x55af9e590337]
> > > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > > [0x7fd31ef99b97]
> > > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > > > fault
> > > > > > >
> > > > > > > I can reproduce this on both x86 and ARM64.
> > > > > >
> > > > > > I don't see this with these two csets removed:
> > > > > >
> > > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > > > perf_event_paranoid checks
> > > > > >
> > > > > > Which were the ones I guessed were related to the problem you
> > > > > > reported, so they are out of my ongoing perf/core pull request
> > > > > > to Ingo/Thomas, now trying with these applied and your
> instructions...
> > > > >
> > > > > Can't repro:
> > > > >
> > > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > > 0
> > > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> record:
> > > > > Woken up 1 times to write data ] [ perf record: Captured and
> > > > > wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > > instructions:k uname Linux [ perf record: Woken up 1 times to write
> data ] [ perf record:
> > > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco
> > > > > ~]# echo
> > > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record
> > > > > -e instructions:k uname Linux [ perf record: Woken up 1 times to
> > > > > write data ] [ perf record: Captured and wrote 0.024 MB
> > > > > perf.data (1 samples) ] [root@quaco ~]#
> > > > >
> > > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with
> > > > > kptr_restrict checks d7604b66102e perf tools: Use CAP_SYS_ADMIN
> > > > > with perf_event_paranoid checks c766f3df635d perf ftrace: Use
> > > > > CAP_SYS_ADMIN instead of euid==0 c22e150e3afa perf tools: Add
> > > > > helpers to use capabilities if present
> > > > > 74d5f3d06f70 tools build: Add capability-related feature
> > > > > detection perf version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > > >
> > > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > > # cat /proc/sys/kernel/kptr_restrict
> > > > 0
> > > >
> > > > Perf trace works fine (does not die):
> > > > # ./perf trace -a
> > > >
> > > > Here is my setup:
> > > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > > Branch: tmp.perf/cap
> > > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict
> checks"
> > > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10
> > > > 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux lsb_release
> > > > -a: Ubuntu 18.04.3 LTS
> > > >
> > > > Auto-detecting system features:
> > > > ... dwarf: [ on ]
> > > > ... dwarf_getlocations: [ on ]
> > > > ... glibc: [ on ]
> > > > ... gtk2: [ on ]
> > > > ... libaudit: [ on ]
> > > > ... libbfd: [ on ]
> > > > ... libcap: [ on ]
> > > > ... libelf: [ on ]
> > > > ... libnuma: [ on ]
> > > > ... numa_num_possible_cpus: [ on ]
> > > > ... libperl: [ on ]
> > > > ... libpython: [ on ]
> > > > ... libcrypto: [ on ]
> > > > ... libunwind: [ on ]
> > > > ... libdw-dwarf-unwind: [ on ]
> > > > ... zlib: [ on ]
> > > > ... lzma: [ on ]
> > > > ... get_cpuid: [ OFF ]
> > > > ... bpf: [ on ]
> > > > ... libaio: [ on ]
> > > > ... libzstd: [ on ]
> > > > ... disassembler-four-args: [ on ]
> > > >
> > > > I also could not reproduce on x86:
> > > > lsb_release -a: Ubuntu 18.04.1 LTS gcc --version: gcc (Ubuntu
> > > > 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0 uname -r: 4.4.0-154-generic
> > >
> > > I isolated the problem to libcap-dev - if it is not installed a
> > > segmentation fault will occur. Since this set is about using
> > > capabilities it is obvious that not having it on a system should
> > > fail a trace session, but it should not crash it.
> >
> > It shouldn't crash on x86_64:
> >
> > root@quaco ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0
> > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > record: Woken up 1 times to write data ] [ perf record: Captured and
> > wrote 0.023 MB perf.data (5 samples) ] [root@quaco ~]# ldd ~/bin/perf
> > | grep libcap [root@quaco ~]# perf -v perf version
> > 5.3.rc4.g329ca330bf8b [root@quaco ~]# [acme@quaco perf]$ git log
> > --oneline -4 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use
> > CAP_SYSLOG with kptr_restrict checks f7b9999387af perf tools: Use
> > CAP_SYS_ADMIN with perf_event_paranoid checks
> > 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf
> > report: Add --switch-on/--switch-off events
> > 2f53ae347f59 perf top: Add --switch-on/--switch-off events [acme@quaco
> > perf]$
> >
> > > If libcap-dev is not installed function
> > > symbol__restricted_filename() will return true, which in turn will
> > > prevent symbol_name to be set in
> > > machine__get_running_kernel_start(). That prevents function
> > > map__set_kallsyms_ref_reloc_sym() from being called in
> > > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym
> > > being NULL in _perf_event__synthesize_kernel_mmap() and a
> > > segmentation fault.
> >
> > Can you please try with my perf/cap branch? Perhaps you're using
> > Igor's original set of patches? I made changes to the fallback, he was
> > making it return false while I made it fallback to geteuid() == 0, as
> > was before his patches.
>
> Things are working properly on your perf/cap branch. I tested with on both
> x86 and ARM.

Mathieu, you are probably testing with euid==0. If you were to test with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0, perf_event_paranoid=2), you would likely hit the bug that you identified in __perf_event__synthesize_kermel_mmap().

See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).

Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.

- Igor

> > > I am not sure how this can be fixed. I counted a total of 19
> > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > guaranteed to be valid. If I am correct then all we need is to
> > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > Otherwise it will be a little harder.
> > >
> > > Mathieu

2019-08-20 16:59:24

[permalink] [raw]

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

On Mon, 19 Aug 2019 at 16:22, Lubashev, Igor <[email protected]> wrote:
>
> On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <[email protected]> wrote:
> > On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> > >
> > > Em Thu, Aug 15, 2019 at 02:16:48PM -0600, Mathieu Poirier escreveu:
> > > > On Wed, 14 Aug 2019 at 14:02, Lubashev, Igor <[email protected]>
> > wrote:
> > > > >
> > > > > > On Wed, August 14, 2019 at 2:52 PM Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> > > > > > Em Wed, Aug 14, 2019 at 03:48:14PM -0300, Arnaldo Carvalho de
> > > > > > Melo
> > > > > > escreveu:
> > > > > > > Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier
> > escreveu:
> > > > > > > > # echo 0 > /proc/sys/kernel/kptr_restrict #
> > > > > > > > ./tools/perf/perf record -e instructions:k uname
> > > > > > > > perf: Segmentation fault
> > > > > > > > Obtained 10 stack frames.
> > > > > > > > ./tools/perf/perf(sighandler_dump_stack+0x44)
> > > > > > > > [0x55af9e5da5d4]
> > > > > > > > /lib/x86_64-linux-gnu/libc.so.6(+0x3ef20) [0x7fd31efb6f20]
> > > > > > > > ./tools/perf/perf(perf_event__synthesize_kernel_mmap+0xa7)
> > > > > > > > [0x55af9e590337]
> > > > > > > > ./tools/perf/perf(+0x1cf5be) [0x55af9e50c5be]
> > > > > > > > ./tools/perf/perf(cmd_record+0x1022) [0x55af9e50dff2]
> > > > > > > > ./tools/perf/perf(+0x23f98d) [0x55af9e57c98d]
> > > > > > > > ./tools/perf/perf(+0x23fc9e) [0x55af9e57cc9e]
> > > > > > > > ./tools/perf/perf(main+0x369) [0x55af9e4f6bc9]
> > > > > > > > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)
> > > > > > > > [0x7fd31ef99b97]
> > > > > > > > ./tools/perf/perf(_start+0x2a) [0x55af9e4f704a] Segmentation
> > > > > > > > fault
> > > > > > > >
> > > > > > > > I can reproduce this on both x86 and ARM64.
> > > > > > >
> > > > > > > I don't see this with these two csets removed:
> > > > > > >
> > > > > > > 7ff5b5911144 perf symbols: Use CAP_SYSLOG with kptr_restrict
> > > > > > > checks d7604b66102e perf tools: Use CAP_SYS_ADMIN with
> > > > > > > perf_event_paranoid checks
> > > > > > >
> > > > > > > Which were the ones I guessed were related to the problem you
> > > > > > > reported, so they are out of my ongoing perf/core pull request
> > > > > > > to Ingo/Thomas, now trying with these applied and your
> > instructions...
> > > > > >
> > > > > > Can't repro:
> > > > > >
> > > > > > [root@quaco ~]# cat /proc/sys/kernel/kptr_restrict
> > > > > > 0
> > > > > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > record:
> > > > > > Woken up 1 times to write data ] [ perf record: Captured and
> > > > > > wrote 0.024 MB perf.data (1 samples) ] [root@quaco ~]# echo 1 >
> > > > > > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record -e
> > > > > > instructions:k uname Linux [ perf record: Woken up 1 times to write
> > data ] [ perf record:
> > > > > > Captured and wrote 0.024 MB perf.data (1 samples) ] [root@quaco
> > > > > > ~]# echo
> > > > > > 0 > /proc/sys/kernel/kptr_restrict [root@quaco ~]# perf record
> > > > > > -e instructions:k uname Linux [ perf record: Woken up 1 times to
> > > > > > write data ] [ perf record: Captured and wrote 0.024 MB
> > > > > > perf.data (1 samples) ] [root@quaco ~]#
> > > > > >
> > > > > > [acme@quaco perf]$ git log --oneline --author Lubashev tools/
> > > > > > 7ff5b5911144 (HEAD -> perf/cap, acme.korg/tmp.perf/cap,
> > > > > > acme.korg/perf/cap) perf symbols: Use CAP_SYSLOG with
> > > > > > kptr_restrict checks d7604b66102e perf tools: Use CAP_SYS_ADMIN
> > > > > > with perf_event_paranoid checks c766f3df635d perf ftrace: Use
> > > > > > CAP_SYS_ADMIN instead of euid==0 c22e150e3afa perf tools: Add
> > > > > > helpers to use capabilities if present
> > > > > > 74d5f3d06f70 tools build: Add capability-related feature
> > > > > > detection perf version 5.3.rc4.g7ff5b5911144 [acme@quaco perf]$
> > > > >
> > > > > I got an ARM64 cloud VM, but I cannot reproduce.
> > > > > # cat /proc/sys/kernel/kptr_restrict
> > > > > 0
> > > > >
> > > > > Perf trace works fine (does not die):
> > > > > # ./perf trace -a
> > > > >
> > > > > Here is my setup:
> > > > > Repo: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
> > > > > Branch: tmp.perf/cap
> > > > > Commit: 7ff5b5911 "perf symbols: Use CAP_SYSLOG with kptr_restrict
> > checks"
> > > > > gcc --version: gcc (Ubuntu/Linaro 7.4.0-1ubuntu1~18.04.1) 7.4.0
> > > > > uname -a: Linux arm-4-par-1 4.9.93-mainline-rev1 #1 SMP Tue Apr 10
> > > > > 09:54:46 UTC 2018 aarch64 aarch64 aarch64 GNU/Linux lsb_release
> > > > > -a: Ubuntu 18.04.3 LTS
> > > > >
> > > > > Auto-detecting system features:
> > > > > ... dwarf: [ on ]
> > > > > ... dwarf_getlocations: [ on ]
> > > > > ... glibc: [ on ]
> > > > > ... gtk2: [ on ]
> > > > > ... libaudit: [ on ]
> > > > > ... libbfd: [ on ]
> > > > > ... libcap: [ on ]
> > > > > ... libelf: [ on ]
> > > > > ... libnuma: [ on ]
> > > > > ... numa_num_possible_cpus: [ on ]
> > > > > ... libperl: [ on ]
> > > > > ... libpython: [ on ]
> > > > > ... libcrypto: [ on ]
> > > > > ... libunwind: [ on ]
> > > > > ... libdw-dwarf-unwind: [ on ]
> > > > > ... zlib: [ on ]
> > > > > ... lzma: [ on ]
> > > > > ... get_cpuid: [ OFF ]
> > > > > ... bpf: [ on ]
> > > > > ... libaio: [ on ]
> > > > > ... libzstd: [ on ]
> > > > > ... disassembler-four-args: [ on ]
> > > > >
> > > > > I also could not reproduce on x86:
> > > > > lsb_release -a: Ubuntu 18.04.1 LTS gcc --version: gcc (Ubuntu
> > > > > 7.4.0-1ubuntu1~18.04aka10.0.0) 7.4.0 uname -r: 4.4.0-154-generic
> > > >
> > > > I isolated the problem to libcap-dev - if it is not installed a
> > > > segmentation fault will occur. Since this set is about using
> > > > capabilities it is obvious that not having it on a system should
> > > > fail a trace session, but it should not crash it.
> > >
> > > It shouldn't crash on x86_64:
> > >
> > > root@quaco ~]# sysctl kernel.kptr_restrict kernel.kptr_restrict = 0
> > > [root@quaco ~]# perf record -e instructions:k uname Linux [ perf
> > > record: Woken up 1 times to write data ] [ perf record: Captured and
> > > wrote 0.023 MB perf.data (5 samples) ] [root@quaco ~]# ldd ~/bin/perf
> > > | grep libcap [root@quaco ~]# perf -v perf version
> > > 5.3.rc4.g329ca330bf8b [root@quaco ~]# [acme@quaco perf]$ git log
> > > --oneline -4 329ca330bf8b (HEAD -> perf/cap) perf symbols: Use
> > > CAP_SYSLOG with kptr_restrict checks f7b9999387af perf tools: Use
> > > CAP_SYS_ADMIN with perf_event_paranoid checks
> > > 4d0489cf1c47 (acme.korg/tmp.perf/script-switch-on-off, perf/core) perf
> > > report: Add --switch-on/--switch-off events
> > > 2f53ae347f59 perf top: Add --switch-on/--switch-off events [acme@quaco
> > > perf]$
> > >
> > > > If libcap-dev is not installed function
> > > > symbol__restricted_filename() will return true, which in turn will
> > > > prevent symbol_name to be set in
> > > > machine__get_running_kernel_start(). That prevents function
> > > > map__set_kallsyms_ref_reloc_sym() from being called in
> > > > machine__create_kernel_maps(), resulting in kmap->ref_reloc_sym
> > > > being NULL in _perf_event__synthesize_kernel_mmap() and a
> > > > segmentation fault.
> > >
> > > Can you please try with my perf/cap branch? Perhaps you're using
> > > Igor's original set of patches? I made changes to the fallback, he was
> > > making it return false while I made it fallback to geteuid() == 0, as
> > > was before his patches.
> >
> > Things are working properly on your perf/cap branch. I tested with on both
> > x86 and ARM.
>
> Mathieu, you are probably testing with euid==0.

Very true

> If you were to test with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0, perf_event_paranoid=2), you would likely hit the bug that you identified in __perf_event__synthesize_kermel_mmap().
>
> See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).
>
> Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.

CC me on the next patchset and I'll be happy to test it.

>
> - Igor
>
>
> > > > I am not sure how this can be fixed. I counted a total of 19
> > > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > > guaranteed to be valid. If I am correct then all we need is to
> > > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > > Otherwise it will be a little harder.
> > > >
> > > > Mathieu
>

2019-08-20 17:16:28

[permalink] [raw]

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

Em Mon, Aug 19, 2019 at 10:22:07PM +0000, Lubashev, Igor escreveu:
> On Mon, August 19, 2019 at 12:51 PM Mathieu Poirier <[email protected]> wrote:
> > On Thu, 15 Aug 2019 at 15:42, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Things are working properly on your perf/cap branch. I tested with on both
> > x86 and ARM.

> Mathieu, you are probably testing with euid==0. If you were to test
> with euid!=0 but with CAP_SYSLOG and no libcap (and kptr_restrict=0,
> perf_event_paranoid=2), you would likely hit the bug that you
> identified in __perf_event__synthesize_kermel_mmap().

> See https://lkml.kernel.org/lkml/930a59730c0d495f8c5acf4f99048e8d@usma1ex-dag1mb6.msg.corp.akamai.com for the fix (Option 1 only or Options 1+2).
>
> Arnaldo, once we decide what the right fix is, I am happy to post the update (options 1, 1+2) as a patch series.

I think you should get the checks for ref_reloc_sym in place so as to
make the code overall more robust, and also go on continuing to make the
checks in tools/perf/ to match what is checked on the other side of the
mirror, i.e. by the kernel, so from a quick read, please put first the
robustness patches (check ref_reloc_sym) do your other suggestions and
update the warnings, then refresh the two patches that still are not in
my perf/core branch:

[acme@quaco perf]$ git rebase perf/core
First, rewinding head to replay your work on top of it...
Applying: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
Applying: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
[acme@quaco perf]$

I've pushed out perf/cap, so you can go from there as it is rebased on
my current perf/core.

Then test all these cases: with/without libcap, with euid==0 and
different than zero, with capabilities, etc, patch by patch so that we
don't break bisection nor regress,

Thanks and keep up the good work!

- Arnaldo

> - Igor
>
>
> > > > I am not sure how this can be fixed. I counted a total of 19
> > > > instances where kmap->ref_reloc_sym->XYZ is called, only 2 of wich
> > > > care to check if kmap->ref_reloc_sym is valid before proceeding. As
> > > > such I must hope that in the 17 other cases, kmap->ref_reloc_sym is
> > > > guaranteed to be valid. If I am correct then all we need is to
> > > > check for a valid pointer in _perf_event__synthesize_kernel_mmap().
> > > > Otherwise it will be a little harder.
> > > >
> > > > Mathieu
>

--

- Arnaldo

2019-08-27 02:00:06