linux-kernel - Re: [PATCH V5 1/3] riscv: Add perf callchain support

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: <CAEbi=3dtDy=CMRagVrj0ihtpYqS+4NkK7eYmn6Gn=2bV9khWVg@mail.gmail.com>
Date:   Mon, 26 Aug 2019 16:03:07 +0800
From:   Greentime Hu <green.hu@...il.com>
To:     Guo Ren <guoren@...nel.org>, greentime.hu@...ive.com
Cc:     Mao Han <han_mao@...ky.com>, linux-riscv@...ts.infradead.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...ive.com>,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH V5 1/3] riscv: Add perf callchain support

Hi Guo,

Guo Ren <guoren@...nel.org> 於 2019年8月24日 週六 上午8:54寫道：
>
> Please check CONFIG_FRAME_POINTER
>
> 1 *frame = *((struct stackframe *)frame->fp - 1);
> This code is origionally from riscv/kernel/stacktrace.c: walk_stackframe
>
> In linux/Makefile it'll involve the options for gcc to definitely
> store ra & prev_fp in fp pointer.
> ifdef CONFIG_FRAME_POINTER
> KBUILD_CFLAGS += -fno-omit-frame-pointer -fno-optimize-sibling-calls
>
> So --call-graph fp need depends on CONFIG_FRAME_POINTER.
>

I am pretty sure CONFIG_FRAME_POINTER is Y
# zcat /proc/config.gz |grep CONFIG_FRAME_POINTER
CONFIG_FRAME_POINTER=y

This is not going to go wrong every time, the probability of error is
about one tenth or one quarter. randomly
There may be some conditions that we have not considered.

I add one more condition to check if it is a valid virtual address and
it( ./perf record -e cpu-clock --call-graph fp ls) passes 1000 times
without failure in Unleashed board based on 5.3-rc5.
Here is my patch. Please have  a look at it. I am not sure if it is a
good solution.

diff --git a/arch/riscv/kernel/perf_callchain.c
b/arch/riscv/kernel/perf_callchain.c
index d75d15c13dc7..4717942435df 100644
--- a/arch/riscv/kernel/perf_callchain.c
+++ b/arch/riscv/kernel/perf_callchain.c
@@ -18,6 +18,8 @@ static int unwind_frame_kernel(struct stackframe *frame)
                return -EPERM;
        if (frame->fp < CONFIG_PAGE_OFFSET)
                return -EPERM;
+       if (!virt_addr_valid(frame->fp))
+               return -EPERM;

        *frame = *((struct stackframe *)frame->fp - 1);
        if (__kernel_text_address(frame->ra)) {

It could catch cases called in this way.

[ 1381.936586] frame->fp=:ffffffff00547550
[ 1382.038542] CPU: 1 PID: 135 Comm: ls Not tainted
5.3.0-rc5-00003-gb008f6bcd67c-dirty #14
[ 1382.307440] Call Trace:
[ 1382.388947] [<ffffffe0002a2d8e>] walk_stackframe+0x0/0x9a
[ 1382.568053] [<ffffffe0002a2f5a>] show_stack+0x2a/0x34
[ 1382.735960] [<ffffffe00083dcd6>] dump_stack+0x62/0x7c
[ 1382.903863] [<ffffffe0002a49e0>] perf_callchain_kernel+0xd8/0x102
[ 1383.106558] [<ffffffe000340a6e>] get_perf_callchain+0x136/0x1f2
[ 1383.303128] [<ffffffe00033d480>] perf_callchain+0x52/0x6e
[ 1383.482553] [<ffffffe00033d50a>] perf_prepare_sample+0x6e/0x476
[ 1383.679357] [<ffffffe00033d92e>] perf_event_output_forward+0x1c/0x50
[ 1383.890633] [<ffffffe000338b4c>] __perf_event_overflow+0x6a/0xa4
[ 1384.090279] [<ffffffe000338c40>] perf_swevent_hrtimer+0xba/0x106
[ 1384.290094] [<ffffffe0002f307c>] __hrtimer_run_queues+0x84/0x108
[ 1384.489694] [<ffffffe0002f36b8>] hrtimer_interrupt+0xca/0x1ce
[ 1384.680974] [<ffffffe00072f572>] riscv_timer_interrupt+0x32/0x3a
[ 1384.880449] [<ffffffe000854b34>] do_IRQ+0x64/0xbe
[ 1385.036698] [<ffffffe0002a1c5c>] ret_from_exception+0x0/0xc

[13915.697989] frame->fp=:fffffffffffff000
[13915.799937] CPU: 2 PID: 663 Comm: ls Not tainted
5.3.0-rc5-00003-gb008f6bcd67c-dirty #14
[13916.068832] Call Trace:
[13916.150380] [<ffffffe0002a2d8e>] walk_stackframe+0x0/0x9a
[13916.329450] [<ffffffe0002a2f5a>] show_stack+0x2a/0x34
[13916.497360] [<ffffffe00083dcd6>] dump_stack+0x62/0x7c
[13916.665265] [<ffffffe0002a49e0>] perf_callchain_kernel+0xd8/0x102
[13916.867949] [<ffffffe000340a6e>] get_perf_callchain+0x136/0x1f2
[13917.064526] [<ffffffe00033d480>] perf_callchain+0x52/0x6e
[13917.243950] [<ffffffe00033d50a>] perf_prepare_sample+0x6e/0x476
[13917.440759] [<ffffffe00033d92e>] perf_event_output_forward+0x1c/0x50
[13917.652021] [<ffffffe000338b4c>] __perf_event_overflow+0x6a/0xa4
[13917.851683] [<ffffffe000338c40>] perf_swevent_hrtimer+0xba/0x106
[13918.051494] [<ffffffe0002f307c>] __hrtimer_run_queues+0x84/0x108
[13918.251094] [<ffffffe0002f36b8>] hrtimer_interrupt+0xca/0x1ce
[13918.442379] [<ffffffe00072f572>] riscv_timer_interrupt+0x32/0x3a
[13918.641840] [<ffffffe000854b34>] do_IRQ+0x64/0xbe
[13918.798082] [<ffffffe0002a1c5c>] ret_from_exception+0x0/0xc