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

[Arnaldo Carvalho de Melo]

Em Wed, Aug 14, 2019 at 12:04:33PM -0600, Mathieu Poirier escreveu:
> On Wed, 7 Aug 2019 at 08:44, Igor Lubashev <ilubashe@xxxxxxxxxx> wrote:
> >
> > Kernel is using CAP_SYSLOG capability instead of uid==0 and euid==0 when
> > checking kptr_restrict. Make perf do the same.
> >
> > Also, the kernel is a more restrictive than "no restrictions" in case of
> > kptr_restrict==0, so add the same logic to perf.
> >
> > Signed-off-by: Igor Lubashev <ilubashe@xxxxxxxxxx>
> > ---
> >  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