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

From: Andy Lutomirski
Date: Mon Mar 09 2015 - 11:16:33 EST


On Mon, Mar 9, 2015 at 7:52 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
> 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@xxxxxxxxxxxxxx>

>
> 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
> ---
> 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
--
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/