diff mbox

perf record: Fix segfault when running with suid and kptr_restrict is 1

Message ID 1474429700-130812-1-git-send-email-wangnan0@huawei.com
State Superseded
Headers show

Commit Message

Wang Nan Sept. 21, 2016, 3:48 a.m. UTC
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@huawei.com>

Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

-- 
1.8.3.4

Comments

Wang Nan Sept. 21, 2016, 3:55 a.m. UTC | #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@huawei.com>

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

>
diff mbox

Patch

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