linux-kernel - Re: [PATCH] arm64: stacktrace: don't trace arch_stack

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: <20210322130158.GA78652@C02TD0UTHF1T.local>
Date:   Mon, 22 Mar 2021 13:01:58 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Chen Jun <chenjun102@...wei.com>,
        Marco Elver <elver@...gle.com>, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH] arm64: stacktrace: don't trace arch_stack_walk()

On Fri, Mar 19, 2021 at 07:02:06PM +0000, Catalin Marinas wrote:
> On Fri, Mar 19, 2021 at 06:41:06PM +0000, Mark Rutland wrote:
> > We recently converted arm64 to use arch_stack_walk() in commit:
> > 
> >   5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK")
> > 
> > The core stacktrace code expects that (when tracing the current task)
> > arch_stack_walk() starts a trace at its caller, and does not include
> > itself in the trace. However, arm64's arch_stack_walk() includes itself,
> > and so traces include one more entry than callers expect. The core
> > stacktrace code which calls arch_stack_walk() tries to skip a number of
> > entries to prevent itself appearing in a trace, and the additional entry
> > prevents skipping one of the core stacktrace functions, leaving this in
> > the trace unexpectedly.
> > 
> > We can fix this by having arm64's arch_stack_walk() begin the trace with
> > its caller. The first value returned by the trace will be
> > __builtin_return_address(0), i.e. the caller of arch_stack_walk(). The
> > first frame record to be unwound will be __builtin_frame_address(1),
> > i.e. the caller's frame record. To prevent surprises, arch_stack_walk()
> > is also marked noinline.

[...]

> > Fixes: 5fc57df2f6fd ("arm64: stacktrace: Convert to ARCH_STACKWALK")
> > Signed-off-by: Mark Rutland <mark.rutland@....com>
> > Cc: Catalin Marinas <catalin.marinas@....com>
> > Cc: Chen Jun <chenjun102@...wei.com>
> > Cc: Marco Elver <elver@...gle.com>
> > Cc: Mark Brown <broonie@...nel.org>
> > Cc: Will Deacon <will@...nel.org>
> 
> Thanks Mark. I think we should add a cc stable, just with Fixes doesn't
> always seem to end up in a stable kernel:
> 
> Cc: <stable@...r.kernel.org> # 5.10.x

Makes sense to me, sure.

> With that:
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@....com>

Thanks!

Will, I assume you're happy to fold in the above when picking this. If
you'd prefer I repost with that folded in, please let me know!

Mark.