linux-kernel - Re: [PATCH v4 2/4] x86/virt: Force GIF=1 prior to disabling SVM (for reboot flows)

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]

Message-ID: <e3016964-f1d5-f064-134e-d9e00531bdce@citrix.com>
Date:   Fri, 2 Dec 2022 00:08:22 +0000
From:   Andrew Cooper <Andrew.Cooper3@...rix.com>
To:     Sean Christopherson <seanjc@...gle.com>
CC:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Guilherme G . Piccoli" <gpiccoli@...lia.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        "stable@...r.kernel" <stable@...r.kernel>,
        Andrew Cooper <Andrew.Cooper3@...rix.com>
Subject: Re: [PATCH v4 2/4] x86/virt: Force GIF=1 prior to disabling SVM (for
 reboot flows)

On 01/12/2022 23:04, Sean Christopherson wrote:
> On Thu, Dec 01, 2022, Andrew Cooper wrote:
>> On 30/11/2022 23:36, Sean Christopherson wrote:
>>> diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h
>>> index 8757078d4442..0acb14806a74 100644
>>> --- a/arch/x86/include/asm/virtext.h
>>> +++ b/arch/x86/include/asm/virtext.h
>>> @@ -126,7 +126,18 @@ static inline void cpu_svm_disable(void)
>>>  
>>>  	wrmsrl(MSR_VM_HSAVE_PA, 0);
>>>  	rdmsrl(MSR_EFER, efer);
>>> -	wrmsrl(MSR_EFER, efer & ~EFER_SVME);
>>> +	if (efer & EFER_SVME) {
>>> +		/*
>>> +		 * Force GIF=1 prior to disabling SVM, e.g. to ensure INIT and
>>> +		 * NMI aren't blocked.  Eat faults on STGI, as it #UDs if SVM
>>> +		 * isn't enabled and SVM can be disabled by an NMI callback.
>> I'd be tempted to tweak this for clarity.
>>
>> How about "We don't know the state of GIF, and if NMIs are enabled,
>> there is a race condition where EFER.SVME can be cleared behind our
>> back.  Ignore #UD, and force GIF=1 in case INIT/NMI are currently
>> blocked."  ?
>>
>> The STGI can't actually #UD on real hardware, because SKINIT and SVM
>> exist in identical sets of parts, but it can #UD in principle in a VM
>> which doesn't offer emulate SKINIT.
> Ah, right, "may #UD", not "will #UD".  And despite writing this, I also keep
> forgetting why forcing GIF is even necessary.  How about?
>
> 		/*
> 		 * Force GIF=1 prior to disabling SVM to ensure INIT and NMI
> 		 * aren't blocked, e.g. if a fatal error occurred between CLGI
> 		 * and STGI.  Note, STGI may #UD if SVM is disabled from NMI
> 		 * context between reading EFER and executing STGI.  In that
> 		 * case, GIF must already be set, otherwise the NMI would have
> 		 * been blocked, so just eat the fault.
> 		 */

LGTM.

~Andrew