linux-kernel - Re: [PATCH v2 13/18] x86/asm/64: Use a percpu trampoline stack for IDT entries

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:   Fri, 24 Nov 2017 00:44:34 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andy Lutomirski <luto@...nel.org>
cc:     X86 ML <x86@...nel.org>, Borislav Petkov <bpetkov@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: [PATCH v2 13/18] x86/asm/64: Use a percpu trampoline stack for
 IDT entries

On Tue, 21 Nov 2017, Andy Lutomirski wrote:
> The asm isn't exactly beautiful,

Delightful euphemism :)

> but I think that fully refactoring
> it can wait.

> @@ -560,6 +560,14 @@ END(irq_entries_start)
>  	.macro interrupt func
>  	cld
>  	ALLOC_PT_GPREGS_ON_STACK
> +
> +	testb	$3, CS(%rsp)
> +	jz	1f
> +	SWAPGS
> +	call	switch_to_thread_stack
> +	SWAPGS

I'm surely missing something subtle, but the register saving does really
not care on which GS it is. This swapgs orgy looks odd.

> +1:
> +
>  	SAVE_C_REGS
>  	SAVE_EXTRA_REGS
>  	ENCODE_FRAME_POINTER
> @@ -827,6 +835,33 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
>   */
>  #define CPU_TSS_IST(x) PER_CPU_VAR(cpu_tss) + (TSS_ist + ((x) - 1) * 8)
>  
> +/*
> + * Switch to the thread stack.  This is called with the IRET frame and
> + * orig_ax in pt_regs and the rest of pt_regs allocated, but with all GPRs
> + * in the CPU registers.

That took several attempts to grok why you left ALLOC_PT_GPRES_ON_STACK in
place in the interrupts macro above.

In theory it would be sufficient to push %rdi on the entry stack and
operate from there, but it spares only the 'addq %rsp'. Not worth the
trouble of dealing with different register offsets.

A comment to that effect would be useful if you look at that 3 month from
now.

> + */
> +ENTRY(switch_to_thread_stack)
> +	UNWIND_HINT_IRET_REGS offset=17*8
> +
> +	movq	%rdi, RDI+8(%rsp)
> +	movq	%rsp, %rdi
> +	movq	PER_CPU_VAR(cpu_current_top_of_stack), %rsp
> +	UNWIND_HINT_IRET_REGS offset=17*8 base=%rdi
> +
> +	pushq	SS+8(%rdi)		/* regs->ss */
> +	pushq	RSP+8(%rdi)		/* regs->rsp */
> +	pushq	EFLAGS+8(%rdi)		/* regs->eflags */
> +	pushq	CS+8(%rdi)		/* regs->cs */
> +	pushq	RIP+8(%rdi)		/* regs->ip */
> +	pushq	ORIG_RAX+8(%rdi)	/* regs->orig_ax */
> +	ALLOC_PT_GPREGS_ON_STACK	/* allocate the rest of regs */
> +	pushq	(%rdi)			/* return address */
> +
> +	movq	RDI+8(%rdi), %rdi
> +	UNWIND_HINT_IRET_REGS offset=17*8
> +	ret
> +END(switch_to_thread_stack)

Thanks,

	tglx