linux-kernel - Re: [RFC patch 00/41] stacktrace: Avoid the pointless redirection through struct 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]

Date:   Wed, 10 Apr 2019 13:49:47 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Alexander Potapenko <glider@...gle.com>
Subject: Re: [RFC patch 00/41] stacktrace: Avoid the pointless redirection
 through struct stack_trace

On Wed, Apr 10, 2019 at 12:27:54PM +0200, Thomas Gleixner wrote:
> Struct stack_trace is a sinkhole for input and output parameters which is
> largely pointless for most usage sites. In fact if embedded into other data
> structures it creates indirections and extra storage overhead for no benefit.
> 
> Looking at all usage sites makes it clear that they just require an
> interface which is based on a storage array. That array is either on stack,
> global or embedded into some other data structure.
> 
> Some of the stack depot usage sites are outright wrong, but fortunately the
> wrongness just causes more stack being used for nothing and does not have
> functional impact.
> 
> Another oddity is the inconsistent termination of the stack trace with
> ULONG_MAX. It's pointless as the number of entries is what determines the
> length of the stored trace. In fact quite some call sites remove the
> ULONG_MAX marker afterwards with or without nasty comments about it. Not
> all architectures do that and those which do, do it inconsistenly either
> conditional on nr_entries == 0 or unconditionally.
> 
> The following series cleans that up by:
> 
>     1) Removing the ULONG_MAX termination in the architecture code
> 
>     2) Removing the ULONG_MAX fixups at the call sites
> 
>     3) Providing plain storage array based interfaces for stacktrace and
>        stackdepot.
> 
>     4) Cleaning up the mess at the callsites including some related
>        cleanups.
> 
>     5) Removing the struct stack_trace based interfaces
> 
> This is not changing the struct stack_trace interfaces at the architecture
> level, but it removes the exposure to the generic code.
> 
> It's only lightly tested as I'm traveling and access to my test boxes is
> limited.

This is indeed a much needed cleanup; thanks for starting this.

I didn't spot anything wrong while reading through it, so:

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>