linux-kernel - Re: [PATCH RFC] KVM: nSVM: Fix L1 state corruption upon return from SMM

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Jun 2021 12:38:11 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Michael Roth <mdroth@...ux.vnet.ibm.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Cathy Avery <cavery@...hat.com>,
        Emanuele Giuseppe Esposito <eesposit@...hat.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH RFC] KVM: nSVM: Fix L1 state corruption upon return from
 SMM

On 24/06/21 10:20, Maxim Levitsky wrote:
> Something else to note, just for our information is that KVM
> these days does vmsave/vmload to VM_HSAVE_PA to store/restore
> the additional host state, something that is frowned upon in the spec,
> but there is some justification of doing this in the commit message,
> citing an old spec which allowed this.

True that.  And there is no mention in the specification for VMRUN that 
the host state-save area is a subset of the VMCB format (i.e., that it 
uses VMCB offsets for whatever subset of the state it saves in the 
VMCB), so the spec reference in the commit message is incorrect.  It 
would be nice if the spec guaranteed that.  Michael, Tom?

In fact, Vitaly's patch *will* overwrite the vmsave/vmload parts of 
VM_HSAVE_PA, and it will store the L2 values rather than the L1 values, 
because KVM always does its vmload/vmrun/vmsave sequence using 
vmload(vmcs01) and vmsave(vmcs01)!  So that has to be changed to use 
code similar to svm_set_nested_state (which can be moved to a separate 
function and reused):

         dest->es = src->es;
         dest->cs = src->cs;
         dest->ss = src->ss;
         dest->ds = src->ds;
         dest->gdtr = src->gdtr;
         dest->idtr = src->idtr;
         dest->rflags = src->rflags | X86_EFLAGS_FIXED;
         dest->efer = src->efer;
         dest->cr0 = src->cr0;
         dest->cr3 = src->cr3;
         dest->cr4 = src->cr4;
         dest->rax = src->rax;
         dest->rsp = src->rsp;
         dest->rip = src->rip;
         dest->cpl = 0;

Paolo