linux-kernel - Re: [PATCH] x86/boot: Fix kexec booting failure after SEV early boot support

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:   Tue, 25 Sep 2018 19:26:08 +0200
From:   Borislav Petkov <bp@...e.de>
To:     "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        Kairui Song <kasong@...hat.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "hpa@...or.com" <hpa@...or.com>, "x86@...nel.org" <x86@...nel.org>,
        "Singh, Brijesh" <brijesh.singh@....com>,
        "kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
        "dyoung@...hat.com" <dyoung@...hat.com>
Subject: Re: [PATCH] x86/boot: Fix kexec booting failure after SEV early boot
 support

On Tue, Sep 25, 2018 at 02:33:48PM +0000, Lendacky, Thomas wrote:
> On 09/25/2018 06:10 AM, Kairui Song wrote:
> > Commit 1958b5fc4010 ("x86/boot: Add early boot support when running
> > with SEV active") is causing kexec becomes sometimes unstable, kexec
> > reboot won't start a second kernel bypassing BIOS boot process, instead,
> > the system got reset.
> > 
> > That's because, in get_sev_encryption_bit function, we are using
> > 32-bit RIP-relative addressing to read the value of enc_bit, but
> > kexec may alloc the early boot up code to a higher location, which
> > is beyond 32-bit addressing limit. Some garbage will be read and
> > get_sev_encryption_bit will return the wrong value, which lead to
> > wrong memory page flag.
> > 
> > This patch adds a get_sev_encryption_bit_64 function to avoid this
> > problem. 64-bit early boot code will use this function instead, it
> > uses native RIP addressing to read the enc_bit which have no problem
> > with any location.
> > 
> > Fixes: 1958b5fc4010 ("x86/boot: Add early boot support when running with SEV active")
> > Signed-off-by: Kairui Song <kasong@...hat.com>
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
> 
> > ---
> >  arch/x86/boot/compressed/mem_encrypt.S | 64 ++++++++++++++++++--------
> >  1 file changed, 45 insertions(+), 19 deletions(-)
> > 
> > diff --git a/arch/x86/boot/compressed/mem_encrypt.S b/arch/x86/boot/compressed/mem_encrypt.S
> > index eaa843a52907..41933550449a 100644
> > --- a/arch/x86/boot/compressed/mem_encrypt.S
> > +++ b/arch/x86/boot/compressed/mem_encrypt.S
> > @@ -18,27 +18,13 @@
> >  
> >  	.text
> >  	.code32
> > -ENTRY(get_sev_encryption_bit)
> > +do_get_sev_encryption_bit:
> >  	xor	%eax, %eax
> >  
> >  #ifdef CONFIG_AMD_MEM_ENCRYPT
> >  	push	%ebx
> >  	push	%ecx
> >  	push	%edx
> > -	push	%edi
> > -
> > -	/*
> > -	 * RIP-relative addressing is needed to access the encryption bit
> > -	 * variable. Since we are running in 32-bit mode we need this call/pop
> > -	 * sequence to get the proper relative addressing.
> > -	 */
> > -	call	1f
> > -1:	popl	%edi
> > -	subl	$1b, %edi
> > -
> > -	movl	enc_bit(%edi), %eax
> > -	cmpl	$0, %eax
> > -	jge	.Lsev_exit
> >  
> >  	/* Check if running under a hypervisor */
> >  	movl	$1, %eax
> > @@ -69,25 +55,65 @@ ENTRY(get_sev_encryption_bit)
> >  
> >  	movl	%ebx, %eax
> >  	andl	$0x3f, %eax		/* Return the encryption bit location */
> > -	movl	%eax, enc_bit(%edi)

IINM, the problem can be addressed in a simpler way by getting rid of
enc_bit and thus getting rid of the need to do relative addressing of
anything and simply doing the whole dance of figuring out the C-bit each
time. It probably wouldn't be even measurable...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--