Re: [PATCH] perf record: Fix segfault when running with suid and kptr_restrict is 1

[Wangnan (F)]

On 2016/9/22 23:41, Arnaldo Carvalho de Melo wrote:
> Em Wed, Sep 21, 2016 at 03:56:20AM +0000, Wang Nan escreveu:
> > Before this patch perf panic if kptr_restrict set to 1 and perf is owned
> > by root with suid set:
> >
> >   $ whoami
> >   wangnan
> >   $ ls -l ./perf
> >   -rwsr-xr-x 1 root root 19781908 Sep 21 19:29 /home/wangnan/perf
> >   $ cat /proc/sys/kernel/kptr_restrict
> >   1
> >   $ cat /proc/sys/kernel/perf_event_paranoid
> >   -1
> >   $ ./perf record -a
> >   Segmentation fault (core dumped)
> Trying to reproduce this here, and failing, where am I making a mistake?
>
>   

I tried again. Not related to paranoid. If 
/proc/sys/kernel/kptr_restrict is set to
1 and perf runs with (euid == 0 && uid != 0) it will panic at:

int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
        ...
        kmap = map__kmap(map);
        size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
                        "%s%s", mmap_name, kmap->ref_reloc_sym->name) + 
1; <-- *kmap->ref_reloc_sym is NULL*
        ...
}

because following function:

machine__create_kernel_maps
 machine__get_running_kernel_start

returns NULL, so kmap->ref_reloc_sym is never set.

No-root user never get the crash point because in 
perf_event__synthesize_kernel_mmap()
symbol_conf.kptr_restrict is true so perf_event__synthesize_kernel_mmap 
directly returns
fail.

Have you tried following 'cat' test?

> > The reason is perf assumes it is allowed to read kptr from /proc/kallsyms
> > when euid is root, but in fact kernel doesn't allow it reading kptr when
> > euid and uid are not match with each other:
>   
> >   $ cp /bin/cat .
> >   $ sudo chown root:root ./cat
> >   $ sudo chmod u+s ./cat
> >   $ cat /proc/kallsyms | grep do_fork
> >   0000000000000000 T _do_fork          <--- kptr is hidden even euid is root
> >   $ sudo cat /proc/kallsyms | grep do_fork
> >   ffffffff81080230 T _do_fork
> >
> > See lib/vsprintf.c for kernel side code.
> >
> > This patch fixes this problem by checking both uid and euid.
> Humm, can't we just do:
>
> -			value = (geteuid() != 0) ?
> +			value = (getuid() != 0) ?

No. though not very useful, by chown u+s one can make ((uid == 0) && 
(euid != 0)) by:

 # chown wangnan:wangnan ./perf
 # chmod u+s ./perf

Then perf fail if run by root:
 # ./perf record -a
 Segmentation fault (core dumped)

But okay for normal user.

Thank you.

> I did it here and it seems to work:
>
> [acme@jouet linux]$ whoami
> acme
> [acme@jouet linux]$ ls -la ~/bin/perf
> -rwsr-xr-x. 2 root root 15539952 Sep 22 12:38 /home/acme/bin/perf
> [acme@jouet linux]$ cat /proc/sys/kernel/kptr_restrict
> 1
> [acme@jouet linux]$ cat /proc/sys/kernel/perf_event_paranoid
> -1
> [acme@jouet linux]$ ~acme/bin/perf record -a
>    Warning: File /home/acme/.perfconfig not owned by current user or root, ignoring it.
> WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
> check /proc/sys/kernel/kptr_restrict.
>
> Samples in kernel functions may not be resolved if a suitable vmlinux
> file is not found in the buildid cache or in the vmlinux path.
>
> Samples in kernel modules won't be resolved at all.
>
> If some relocation was applied (e.g. kexec) symbols may be misresolved
> even with a suitable vmlinux or kallsyms file.
>
> Couldn't record kernel reference relocation symbol
> Symbol resolution may be skewed if relocation was used (e.g. kexec).
> Check /proc/kallsyms permission or run as root.
> ^C[ perf record: Woken up 17 times to write data ]
> [ perf record: Captured and wrote 6.474 MB perf.data (90771 samples) ]
>
> [acme@jouet linux]$ perf report -D 2>&1 | grep PERF_RECORD_SAMPLE | wc -l
> 90770
> [acme@jouet linux]$ perf evlist
>    Warning: File /home/acme/.perfconfig not owned by current user or root, ignoring it.
> cycles:ppp
> [acme@jouet linux]$ ls -la ~/.perfconfig
> -rw-rw-r--. 1 acme acme 27 Aug 12 17:57 /home/acme/.perfconfig
> [acme@jouet linux]$
>
>
> - Arnaldo
>
>   
> > Signed-off-by: Wang Nan <wangnan0@xxxxxxxxxx>
> > Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> > ---
> >
> > Resend with a meanless blank like removed.
> >
> > ---
> >   tools/perf/util/symbol.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> > index 19c9c55..c55e781 100644
> > --- a/tools/perf/util/symbol.c
> > +++ b/tools/perf/util/symbol.c
> > @@ -1947,7 +1947,7 @@ static bool symbol__read_kptr_restrict(void)
> >   		char line[8];
> >   
> >   		if (fgets(line, sizeof(line), fp) != NULL)
> > -			value = (geteuid() != 0) ?
> > +			value = ((geteuid() != 0) || (getuid() != 0)) ?
> >   					(atoi(line) != 0) :
> >   					(atoi(line) == 2);
> >   
> > --
> > 1.8.3.4