Re: [PATCH] x86: make 32-bit "emergency stack" better documented

[Andy Lutomirski]

On Mon, Mar 9, 2015 at 6:28 AM, Denys Vlasenko <dvlasenk@xxxxxxxxxx> wrote:
> Before the patch, tss.stack field was not referenced anywhere.
> It was used only by setting sysenter's stack to point after
> last byte of tss, thus the trailing field, stack[64], was used.
> But grep would not know it. You can comment it out, compile,
> and kernel will even run until an unlucky NMI corrupts
> iomap[] (which is also not easily detectable).
>
> This patch changes code so the purpose and usage of this field
> is not mysterious anymore, and can be easily grepped for.
>
> This does change generated code, for a subtle reason:
> since tss_struct is ____cacheline_aligned, there happen to be
> 5 longs of padding at the end. Old code was using the padding too;
> new code will strictly use only SYSENTER_stack[].
>
> Signed-off-by: Denys Vlasenko <dvlasenk@xxxxxxxxxx>
> CC: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> CC: Steven Rostedt <rostedt@xxxxxxxxxxx>
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: Borislav Petkov <bp@xxxxxxxxx>
> CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
> CC: Andy Lutomirski <luto@xxxxxxxxxxxxxx>
> CC: Oleg Nesterov <oleg@xxxxxxxxxx>
> CC: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> CC: Alexei Starovoitov <ast@xxxxxxxxxxxx>
> CC: Will Drewry <wad@xxxxxxxxxxxx>
> CC: Kees Cook <keescook@xxxxxxxxxxxx>
> CC: x86@xxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
>
> ---
>  arch/x86/include/asm/processor.h | 5 +++--
>  arch/x86/kernel/asm-offsets_32.c | 2 +-
>  arch/x86/kernel/cpu/common.c     | 2 +-
>  3 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 48a61c1..4172241 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -276,9 +276,10 @@ struct tss_struct {
>         unsigned long           io_bitmap[IO_BITMAP_LONGS + 1];
>
>         /*
> -        * .. and then another 0x100 bytes for the emergency kernel stack:
> +        * and then space for temporary SYSENTER stack:
>          */
> -       unsigned long           stack[64];
> +       unsigned long           SYSENTER_stack[63];
> +       unsigned long           SYSENTER_stack_last_word;
>
>  } ____cacheline_aligned;
>
> diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
> index 3b3b9d3..4a3cbd0 100644
> --- a/arch/x86/kernel/asm-offsets_32.c
> +++ b/arch/x86/kernel/asm-offsets_32.c
> @@ -68,7 +68,7 @@ void foo(void)
>
>         /* Offset from the sysenter stack to tss.sp0 */
>         DEFINE(TSS_sysenter_sp0, offsetof(struct tss_struct, x86_tss.sp0) -
> -                sizeof(struct tss_struct));
> +               (offsetof(struct tss_struct, SYSENTER_stack_last_word) + 4));

Is there no way to do this using sizeof to avoid having a "last_word" member.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/