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

[Andy Lutomirski <luto@amacapital.net>]

On Mon, Mar 9, 2015 at 7:52 AM, Denys Vlasenko <dvlasenk@redhat.com> 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
> io_bitmap[] (which is also not easily detectable).
>
> This patch changes code so that 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[].

Acked-by: Andy Lutomirski <luto@amacapital.net>

>
> Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
> CC: Linus Torvalds <torvalds@linux-foundation.org>
> CC: Steven Rostedt <rostedt@goodmis.org>
> CC: Ingo Molnar <mingo@kernel.org>
> CC: Borislav Petkov <bp@alien8.de>
> CC: "H. Peter Anvin" <hpa@zytor.com>
> CC: Andy Lutomirski <luto@amacapital.net>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: Frederic Weisbecker <fweisbec@gmail.com>
> CC: Alexei Starovoitov <ast@plumgrid.com>
> CC: Will Drewry <wad@chromium.org>
> CC: Kees Cook <keescook@chromium.org>
> CC: x86@kernel.org
> CC: linux-kernel@vger.kernel.org
> ---
> Changes since v1: use offsetofend()
>
>  arch/x86/include/asm/processor.h | 4 ++--
>  arch/x86/kernel/asm-offsets_32.c | 2 +-
>  arch/x86/kernel/cpu/common.c     | 3 ++-
>  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..9e65cf8 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -276,9 +276,9 @@ 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[64];
>
>  } ____cacheline_aligned;
>
> diff --git a/arch/x86/kernel/asm-offsets_32.c b/arch/x86/kernel/asm-offsets_32.c
> index 3b3b9d3..42a3b28 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));
> +               offsetofend(struct tss_struct, SYSENTER_stack));
>
>  #if defined(CONFIG_LGUEST) || defined(CONFIG_LGUEST_GUEST) || defined(CONFIG_LGUEST_MODULE)
>         BLANK();
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 7634833..4701293 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -987,7 +987,8 @@ void enable_sep_cpu(void)
>         }
>
>         tss->x86_tss.ss1 = __KERNEL_CS;
> -       tss->x86_tss.sp1 = sizeof(struct tss_struct) + (unsigned long) tss;
> +       tss->x86_tss.sp1 = (unsigned long) tss
> +                       + offsetofend(struct tss_struct, SYSENTER_stack);
>         wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
>         wrmsr(MSR_IA32_SYSENTER_ESP, tss->x86_tss.sp1, 0);
>         wrmsr(MSR_IA32_SYSENTER_EIP, (unsigned long) ia32_sysenter_target, 0);
> --
> 1.8.1.4
>



-- 
Andy Lutomirski
AMA Capital Management, LLC