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

["Wangnan (F)" <wangnan0@huawei.com>]

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@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> ---
>>
>> 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