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] [day] [month] [year] [list]

Date:   Wed, 21 Sep 2016 11:55:45 +0800
From:   "Wangnan (F)" <wangnan0@...wei.com>
To:     <acme@...nel.org>
CC:     <linux-kernel@...r.kernel.org>, <lizefan@...wei.com>,
        <pi3orama@....com>, Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH] perf record: Fix segfault when running with suid and
 kptr_restrict is 1



On 2016/9/21 11:48, Wang Nan wrote:
> 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)
>
> 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.
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
>   tools/perf/util/symbol.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 19c9c55..9528702 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1946,8 +1946,9 @@ static bool symbol__read_kptr_restrict(void)
>   	if (fp != NULL) {
>   		char line[8];
>   
> +


Sorry for this blank line. Will fix it.

>   		if (fgets(line, sizeof(line), fp) != NULL)
> -			value = (geteuid() != 0) ?
> +			value = ((geteuid() != 0) || (getuid() != 0)) ?
>   					(atoi(line) != 0) :
>   					(atoi(line) == 2);
>