linux-kernel - Re: [PATCH Part2 RFC v4 24/40] KVM: SVM: Add KVM_SEV_SNP_LAUNCH

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:   Mon, 19 Jul 2021 16:34:48 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        linux-crypto@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        "H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Dov Murik <dovmurik@...ux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@....com>,
        Borislav Petkov <bp@...en8.de>,
        Michael Roth <michael.roth@....com>,
        Vlastimil Babka <vbabka@...e.cz>, tony.luck@...el.com,
        npmccallum@...hat.com, brijesh.ksingh@...il.com
Subject: Re: [PATCH Part2 RFC v4 24/40] KVM: SVM: Add
 KVM_SEV_SNP_LAUNCH_UPDATE command



On 7/19/21 3:51 PM, Sean Christopherson wrote:
> 
> Hmm, and there's no indication on success that the previous entry was assigned?
> Adding a tracepoint in rmpupdate() to allow tracking transitions is probably a
> good idea, otherwise debugging RMP violations and/or unexpected #VC is going to
> be painful.
> 

Absolutely agree. It's in my TODO list for v5. I have been using my 
private debug patches with all those trace debug and will try to pull 
some of those in v5.

> And/or if the kernel/KVM behavior is to never reassign directly and reading an RMP
> entry isn't prohibitively expensive, then we could add a sanity check that the RMP
> is unassigned and reject rmpupdate() if the page is already assigned.  Probably
> not worth it if the overhead is noticeable, but it could be nice to have if things
> go sideways.
> 

In later patches you see that during the page-state change, I do try to 
read RMP entry to detect some of these condition and warn user about 
them. The GHCB specification lets the hypervisor choose how it wants to 
handle the case in guest wanting to add the previously validated page.

> 
> To be clear, it's not just an optimization.  Pages that haven't yet been touched
> may be already owned by a different VM (or even this VM).  I.e. "reverting" those
> pages would actually result in a form of corruption.  It's somewhat of a moot point
> because assigning a single page to multiple guests is going to be fatal anyways,
> but potentially making a bug worse by introducing even more noise/confusion is not
> good.
> 

As you said, if a process is assigning the same page to multiple VMs 
then its fatal but I agree that we should do the right thing from the 
kernel ioctl handling. I will just clear the RMP entry for the pages 
which we touched.

thanks