linux-kernel - Re: [PATCH 0/2] tracing/arm: Fix the stack tracer when LR is saved after local storage

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]

Message-ID: <20190807132058.37616e8f@gandalf.local.home>
Date:   Wed, 7 Aug 2019 13:20:58 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Mark Rutland <mark.rutland@....com>
Cc:     linux-kernel@...r.kernel.org,
        Joel Fernandes <joel@...lfernandes.org>,
        Jiping Ma <jiping.ma2@...driver.com>, mingo@...hat.com,
        catalin.marinas@....com, will.deacon@....com,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 0/2] tracing/arm: Fix the stack tracer when LR is saved
 after local storage

On Wed, 7 Aug 2019 18:08:14 +0100
Mark Rutland <mark.rutland@....com> wrote:

> Hi Steve,
> 
> On Wed, Aug 07, 2019 at 12:34:01PM -0400, Steven Rostedt wrote:
> > As arm64 saves the link register after a function's local variables are
> > stored, it causes the max stack tracer to be off by one in its output
> > of which function has the bloated stack frame.  
> 
> For reference, it's a bit more complex than that. :/

Yeah, I know it is. ;-)

> 
> Our procedure call standard (the AAPCS) says that the frame record may
> be placed anywhere within a stackframe, so we don't have a guarantee as
> to where the saved lr will fall w.r.t local variables.

Yep.

> 
> Today, GCC happens to create the stack frame by creating the stack
> record, so the LR is saved at a lower addresss than the local variables.

Which is what breaks the current algorithm (without this update).

> 
> However, I am aware that there are reasons why a compiler may choose to
> place the frame record at a different locations, e.g. using pointer
> authentication to provide an implicit stack canary, so this could change
> in future, or potentially differ across functions.
> 
> Maybe that's a bridge we'll have to cross in future.

OK, how about I update the change log and add a comment that states
that this can change. But even if it does, it wont break anything but
show the wrong stack size, which is usually only important for us
kernel developers anyway ;-)

Let me send a v2.

-- Steve