linux-kernel - Re: [patch V3 11/32] x86/exceptions: Add structs for exception stacks

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: <20190416182018.GC21674@linux.intel.com>
Date:   Tue, 16 Apr 2019 11:20:18 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [patch V3 11/32] x86/exceptions: Add structs for exception stacks

On Sun, Apr 14, 2019 at 05:59:47PM +0200, Thomas Gleixner wrote:
> At the moment everything assumes a full linear mapping of the various
> exception stacks. Adding guard pages to the cpu entry area mapping of the
> exception stacks will break that assumption.
> 
> As a preparatory step convert both the real storage and the effective
> mapping in the cpu entry area from character arrays to structures.
> 
> To ensure that both arrays have the same ordering and the same size of the
> individual stacks fill the members with a macro. The guard size is the only
> difference between the two resulting structures. For now both have guard
> size 0 until the preparation of all usage sites is done.
> 
> Provide a couple of helper macros which are used in the following
> conversions.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---

One comment nit below, but other than that:

Reviewed-by: Sean Christopherson <sean.j.christopherson@...el.com>

> V2 -> V3: Move the guards below the stacks and add a top guard. Fix the
>       	  TOP() macro.
> ---
>  arch/x86/include/asm/cpu_entry_area.h |   52 ++++++++++++++++++++++++++++++----
>  arch/x86/kernel/cpu/common.c          |    2 -
>  arch/x86/mm/cpu_entry_area.c          |    8 +----
>  3 files changed, 51 insertions(+), 11 deletions(-)
> 
> --- a/arch/x86/include/asm/cpu_entry_area.h
> +++ b/arch/x86/include/asm/cpu_entry_area.h
> @@ -7,6 +7,51 @@
>  #include <asm/processor.h>
>  #include <asm/intel_ds.h>
>  
> +#ifdef CONFIG_X86_64
> +
> +/* Macro to enforce the same ordering and stack sizes */
> +#define ESTACKS_MEMBERS(guardsize)		\
> +	char	DF_stack_guard[guardsize];	\
> +	char	DF_stack[EXCEPTION_STKSZ];	\
> +	char	NMI_stack_guard[guardsize];	\
> +	char	NMI_stack[EXCEPTION_STKSZ];	\
> +	char	DB_stack_guard[guardsize];	\
> +	char	DB_stack[DEBUG_STKSZ];		\
> +	char	MCE_stack_guard[guardsize];	\
> +	char	MCE_stack[EXCEPTION_STKSZ];	\
> +	char	IST_top_guard[guardsize];	\
> +
> +/* The exception stacks linear storage. No guard pages required */

How about s/linear/physical?  "linear" is confusing for us Intel wonks
(or maybe just me) because in Intel's parlance it means virtual addresses
for all intents and purposes.  And an apostrophe to show possession would
help readability, e.g.:

  /* The exception stacks' physical storage. No guard pages required */

> +struct exception_stacks {
> +	ESTACKS_MEMBERS(0)
> +};
>