linux-kernel - Re: [PATCH 6/8] ARM: clang: Do not relay on lr register for stacktrace

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: <CAKwvOdmSX7JcHduY2jF9K80+sFhDNgX0O2DhXCWXYsSNBqPKfA@mail.gmail.com>
Date:   Wed, 13 Oct 2021 12:54:16 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        "Naveen N . Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        Ananth N Mavinakayanahalli <ananth@...ux.ibm.com>,
        Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
        Sven Schnelle <svens@...ux.ibm.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Russell King <linux@...linux.org.uk>,
        Nathan Chancellor <nathan@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Nathan Huckleberry <nhuck@...gle.com>
Subject: Re: [PATCH 6/8] ARM: clang: Do not relay on lr register for stacktrace

On Tue, Oct 12, 2021 at 7:18 AM Masami Hiramatsu <mhiramat@...nel.org> wrote:
>
> Hi Nick,
>
> On Mon, 11 Oct 2021 11:45:22 -0700
> Nick Desaulniers <ndesaulniers@...gle.com> wrote:
>
> >
> > If I change from CONFIG_UNWINDER_ARM=y to
> > CONFIG_UNWINDER_FRAME_POINTER=y, before:
> >
> > # cat /proc/self/stack
> > [<0>] stack_trace_save_tsk+0x50/0x6c
> > [<0>] proc_pid_stack+0xa0/0xf8
> > [<0>] proc_single_show+0x50/0xbc
> > [<0>] seq_read_iter+0x178/0x4ec
> > [<0>] seq_read+0x138/0x15c
> > [<0>] vfs_read+0xd0/0x304
> > [<0>] ksys_read+0x78/0xd4
> > [<0>] sys_read+0xc/0x10
> >
> > after:
> > # cat /proc/self/stack
> > [<0>] proc_pid_stack+0xa0/0xf8
> > [<0>] proc_single_show+0x50/0xbc
> > [<0>] seq_read_iter+0x178/0x4ec
> > [<0>] seq_read+0x138/0x15c
> > [<0>] vfs_read+0xd0/0x304
> > [<0>] ksys_read+0x78/0xd4
> > [<0>] sys_read+0xc/0x10
> > [<0>] __entry_text_start+0x14/0x14
> > [<0>] 0xffffffff
> >
> > So I guess this helps the CONFIG_UNWINDER_FRAME_POINTER=y case? (That
> > final frame address looks wrong, but is potentially yet another bug;
> > perhaps for clang we need to manually store the previous frame's pc at
> > a different offset before jumping to __entry_text_start).
>
> Ah, yes. I didn't touch the UNWINDER_ARM. As you may know the
> unwind_frame()@arch/arm/kernel/stacktrace.c is compiled only
> CONFIG_UNWINDER_FRAME_POINTER=y.

Ah, right, thanks for pointing that out.

> > so likely this fixes/improves CONFIG_UNWINDER_FRAME_POINTER=y? Is that correct?
>
> Yes, that is correct.

In that case, additionally:
Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com>

-- 
Thanks,
~Nick Desaulniers