Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Arvind Sankar <niveditas98@gmail.com>
From:   Arvind Sankar <nivedita@alum.mit.edu>
Date:   Fri, 16 Oct 2020 08:43:01 -0400
To:     Borislav Petkov <bp@alien8.de>
Cc:     Arvind Sankar <nivedita@alum.mit.edu>, x86@kernel.org,
        Joerg Roedel <jroedel@suse.de>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for
 head$(BITS).o
Message-ID: <20201016124301.GA1040839@rani.riverdale.lan>
References: <20201008191623.2881677-1-nivedita@alum.mit.edu>
 <20201008191623.2881677-6-nivedita@alum.mit.edu>
 <20201016111703.GD8483@zn.tnic>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20201016111703.GD8483@zn.tnic>
Precedence: bulk

On Fri, Oct 16, 2020 at 01:17:03PM +0200, Borislav Petkov wrote:
> On Thu, Oct 08, 2020 at 03:16:23PM -0400, Arvind Sankar wrote:
> > On 64-bit, the startup_64_setup_env() function added in
> >   866b556efa12 ("x86/head/64: Install startup GDT")
> > has stack protection enabled because of set_bringup_idt_handler().
> 
> Where? I don't see it.
> 
> I have
> 
> CONFIG_STACKPROTECTOR=y
> # CONFIG_STACKPROTECTOR_STRONG is not set

You need STACKPROTECTOR_STRONG -- I was testing with defconfig and the
option is enabled by default. You also need AMD_MEM_ENCRYPT enabled,
which it looks like you do.

> 
> and a __stack_chk_fail call is nowhere to be found in the resulting
> head64.s file.
> 
> startup_64_setup_env:
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	leaq	startup_gdt(%rdi), %rax	#, tmp99
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	subq	$_text, %rax	#, tmp101
> 	movq	%rax, startup_gdt_descr+2(%rip)	# tmp101, startup_gdt_descr.address
> # ./arch/x86/include/asm/desc.h:209: 	asm volatile("lgdt %0"::"m" (*dtr));
> #APP
> # 209 "./arch/x86/include/asm/desc.h" 1
> 	lgdt startup_gdt_descr(%rip)		# startup_gdt_descr
> # 0 "" 2
> # arch/x86/kernel/head64.c:600: 	asm volatile("movl %%eax, %%ds\n"
> #NO_APP
> 	movl	$24, %eax	#, tmp102
> #APP
> # 600 "arch/x86/kernel/head64.c" 1
> 	movl %eax, %ds
> movl %eax, %ss
> movl %eax, %es
> 
> # 0 "" 2
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> #NO_APP
> 	leaq	bringup_idt_table(%rdi), %r9	#, tmp105
> 	leaq	bringup_idt_descr(%rdi), %r8	#, tmp103
> 	leaq	vc_no_ghcb(%rdi), %rsi	#, tmp107
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	subq	$_text, %r9	#, _11
> 	subq	$_text, %r8	#, _8
> 	subq	$_text, %rsi	#, tmp109
> # arch/x86/kernel/head64.c:572: 		set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
> 	movq	%r9, %rdi	# _11,
> 	call	set_bringup_idt_handler.constprop.0	#
> # arch/x86/kernel/head64.c:575: 	desc->address = (unsigned long)idt;
> 	movq	%r9, 2(%r8)	# _11, MEM[(struct desc_ptr *)_8].address
> # ./arch/x86/include/asm/desc.h:214: 	asm volatile("lidt %0"::"m" (*dtr));
> #APP
> # 214 "./arch/x86/include/asm/desc.h" 1
> 	lidt (%r8)		# MEM[(const struct desc_ptr *)_8]
> # 0 "" 2
> # arch/x86/kernel/head64.c:605: }
> #NO_APP
> 	ret
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette