linux-kernel - Re: [PATCH] perf record: Fix segfault when running with suid and kptr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160922154109.GB5588@kernel.org>
Date:   Thu, 22 Sep 2016 12:41:09 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Wang Nan <wangnan0@...wei.com>
Cc:     linux-kernel@...r.kernel.org, lizefan@...wei.com, pi3orama@....com,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        David Ahern <dsahern@...il.com>,
        Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH] perf record: Fix segfault when running with suid and
 kptr_restrict is 1

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?

 
> 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) ?

I did it here and it seems to work:

[acme@...et linux]$ whoami
acme
[acme@...et linux]$ ls -la ~/bin/perf
-rwsr-xr-x. 2 root root 15539952 Sep 22 12:38 /home/acme/bin/perf
[acme@...et linux]$ cat /proc/sys/kernel/kptr_restrict
1
[acme@...et linux]$ cat /proc/sys/kernel/perf_event_paranoid
-1
[acme@...et 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@...et linux]$ perf report -D 2>&1 | grep PERF_RECORD_SAMPLE | wc -l
90770
[acme@...et linux]$ perf evlist
  Warning: File /home/acme/.perfconfig not owned by current user or root, ignoring it.
cycles:ppp
[acme@...et linux]$ ls -la ~/.perfconfig
-rw-rw-r--. 1 acme acme 27 Aug 12 17:57 /home/acme/.perfconfig
[acme@...et linux]$


- Arnaldo

 
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.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