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

From: Wangnan (F)
Date: Fri Sep 23 2016 - 03:01:10 EST

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
$ ls -l ./perf
-rwsr-xr-x 1 root root 19781908 Sep 21 19:29 /home/wangnan/perf
$ cat /proc/sys/kernel/kptr_restrict
$ cat /proc/sys/kernel/perf_event_paranoid
$ ./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:


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

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@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
[acme@jouet linux]$ cat /proc/sys/kernel/perf_event_paranoid
[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 (90771 samples) ]

[acme@jouet linux]$ perf report -D 2>&1 | grep PERF_RECORD_SAMPLE | wc -l
[acme@jouet linux]$ perf evlist
Warning: File /home/acme/.perfconfig not owned by current user or root, ignoring it.
[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);