kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brijesh Singh <brijesh.singh@amd.com>
To: Dave Hansen <dave.hansen@intel.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Cc: brijesh.singh@amd.com, tglx@linutronix.de, bp@alien8.de,
	jroedel@suse.de, thomas.lendacky@amd.com, pbonzini@redhat.com,
	mingo@redhat.com, rientjes@google.com, seanjc@google.com,
	peterz@infradead.org, hpa@zytor.com, tony.luck@intel.com
Subject: Re: [PATCH Part2 RFC v2 10/37] x86/fault: Add support to handle the RMP fault for kernel address
Date: Mon, 3 May 2021 12:19:34 -0500	[thread overview]
Message-ID: <9e3e4331-2933-7ae6-31d9-5fb73fce4353@amd.com> (raw)
In-Reply-To: <8764e6f0-4a2e-4eea-af69-62ff3ddfe84b@intel.com>


On 5/3/21 11:15 AM, Dave Hansen wrote:
> On 5/3/21 8:37 AM, Brijesh Singh wrote:
>> GHCB was just an example. Another example is a vfio driver accessing the
>> shared page. If those pages are not marked shared then kernel access
>> will cause an RMP fault. Ideally we should not be running into this
>> situation, but if we do, then I am trying to see how best we can avoid
>> the host crashes.
> I'm confused.  Are you suggesting that the VFIO driver could be passed
> an address such that the host kernel would blindly try to write private
> guest memory?

Not blindly. But a guest could trick a VMM (qemu) to ask the host driver
to access a GPA which is guest private page (Its a hypothetical case, so
its possible that I may missing something). Let's see with an example:

- A guest provides a GPA to VMM to write to (e.g DMA operation).

- VMM translates the GPA->HVA and calls down to host kernel with the HVA.

- The host kernel may pin the HVA to get the PFN for it and then kmap().
Write to the mapped PFN will cause an RMP fault if the guest provided
GPA was not a marked shared in the RMP table. In an ideal world, a guest
should *never* do this but what if it does ?


> The host kernel *knows* which memory is guest private and what is
> shared.  It had to set it up in the first place.  It can also consult
> the RMP at any time if it somehow forgot.
>
> So, this scenario seems to be that the host got a guest physical address
> (gpa) from the guest, it did a gpa->hpa->hva conversion and then wrote
> the page all without bothering to consult the RMP.  Shouldn't the the
> gpa->hpa conversion point offer a perfect place to determine if the page
> is shared or private?

The GPA->HVA is typically done by the VMM, and HVA->HPA is done by the
host drivers. So, only time we could verify is after the HVA->HPA. One
of my patch provides a snp_lookup_page_in_rmptable() helper that can be
used to query the page state in the RMP table. This means the all the
host backend drivers need to enlightened to always read the RMP table
before making a write access to guest provided GPA. A good guest should
*never* be using a private page for the DMA operation and if it does
then the fault handler introduced in this patch can avoid the host crash
and eliminate the need to enlightened the drivers to check for the
permission before the access.

I felt it is good idea to have some kind of recovery specially when a
malicious guest could lead us into this path.


>
>> Another reason for having this is to catch  the hypervisor bug, during
>> the SNP guest create, the KVM allocates few backing pages and sets the
>> assigned bit for it (the examples are VMSA, and firmware context page).
>> If hypervisor accidentally free's these pages without clearing the
>> assigned bit in the RMP table then it will result in RMP fault and thus
>> a kernel crash.
> I think I'd be just fine with a BUG_ON() in those cases instead of an
> attempt to paper over the issue.  Kernel crashes are fine in the case of
> kernel bugs.

Yes, fine with me.


>
>>> Or, worst case, you could use exception tables and something like
>>> copy_to_user() to write to the GHCB.  That way, the thread doing the
>>> write can safely recover from the fault without the instruction actually
>>> ever finishing execution.
>>>
>>> BTW, I went looking through the spec.  I didn't see anything about the
>>> guest being able to write the "Assigned" RMP bit.  Did I miss that?
>>> Which of the above three conditions is triggered by the guest failing to
>>> make the GHCB page shared?
>> The GHCB spec section "Page State Change" provides an interface for the
>> guest to request the page state change. During bootup, the guest uses
>> the Page State Change VMGEXIT to request hypervisor to make the page
>> shared. The hypervisor uses the RMPUPDATE instruction to write to
>> "assigned" bit in the RMP table.
> Right...  So the *HOST* is in control.  Why should the host ever be
> surprised by a page transitioning from shared to private?

I am trying is a cover a malicious guest cases. A good guest should
follow the GHCB spec and change the page state before the access.

>
>> On VMGEXIT, the very first thing which vmgexit handler does is to map
>> the GHCB page for the access and then later using the copy_to_user() to
>> sync the GHCB updates from hypervisor to guest. The copy_to_user() will
>> cause a RMP fault if the GHCB is not mapped shared. As I explained
>> above, GHCB page was just an example, vfio or other may also get into
>> this situation.
> Causing an RMP fault is fine.  The problem is shoving a whole bunch of
> *recovery* code in the kernel when recovery isn't necessary.  Just look
> for the -EFAULT from copy_to_user() and move on with life.

  reply	other threads:[~2021-05-03 17:19 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-30 12:37 [PATCH Part2 RFC v2 00/37] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 01/37] KVM: SVM: Add support to handle AP reset MSR protocol Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 02/37] KVM: SVM: Provide the Hypervisor Feature support VMGEXIT Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 03/37] KVM: SVM: Increase the GHCB protocol version Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 04/37] x86/cpufeatures: Add SEV-SNP CPU feature Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 05/37] x86/sev: Add the host SEV-SNP initialization support Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 06/37] x86/sev: Add RMP entry lookup helpers Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 07/37] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 08/37] x86/sev: Split the physmap when adding the page in RMP table Brijesh Singh
2021-05-03 15:07   ` Peter Zijlstra
2021-05-03 15:15   ` Andy Lutomirski
2021-05-03 15:41     ` Dave Hansen
2021-05-07 11:28       ` Vlastimil Babka
2021-04-30 12:37 ` [PATCH Part2 RFC v2 09/37] x86/traps: Define RMP violation #PF error code Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 10/37] x86/fault: Add support to handle the RMP fault for kernel address Brijesh Singh
2021-05-03 14:44   ` Dave Hansen
2021-05-03 15:03     ` Andy Lutomirski
2021-05-03 15:49       ` Brijesh Singh
2021-05-03 15:37     ` Brijesh Singh
2021-05-03 16:15       ` Dave Hansen
2021-05-03 17:19         ` Brijesh Singh [this message]
2021-05-03 17:31           ` Brijesh Singh
2021-05-03 17:40           ` Andy Lutomirski
2021-05-03 19:41             ` Brijesh Singh
2021-05-03 19:43               ` Dave Hansen
2021-05-04 12:31                 ` Brijesh Singh
2021-05-04 14:33                   ` Dave Hansen
2021-05-04 15:16                     ` Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 11/37] x86/fault: Add support to handle the RMP fault for user address Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 12/37] crypto:ccp: Define the SEV-SNP commands Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 13/37] crypto: ccp: Add support to initialize the AMD-SP for SEV-SNP Brijesh Singh
2021-04-30 12:37 ` [PATCH Part2 RFC v2 14/37] crypto: ccp: Shutdown SNP firmware on kexec Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 15/37] crypto:ccp: Provide APIs to issue SEV-SNP commands Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 16/37] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled Brijesh Singh
2021-05-10 18:23   ` Peter Gonda
2021-05-10 20:07     ` Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 17/37] crypto: ccp: Handle the legacy SEV command " Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 18/37] KVM: SVM: make AVIC backing, VMSA and VMCB memory allocation SNP safe Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 19/37] KVM: SVM: Add initial SEV-SNP support Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 20/37] KVM: SVM: define new SEV_FEATURES field in the VMCB Save State Area Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 21/37] KVM: SVM: Add KVM_SNP_INIT command Brijesh Singh
2021-05-06 20:25   ` Peter Gonda
2021-05-06 22:29     ` Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 22/37] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_START command Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 23/37] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_UPDATE command Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 24/37] KVM: SVM: Reclaim the guest pages when SEV-SNP VM terminates Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 25/37] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_FINISH command Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 26/37] KVM: X86: Add kvm_x86_ops to get the max page level for the TDP Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 27/37] KVM: X86: Introduce kvm_mmu_map_tdp_page() for use by SEV Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 28/37] KVM: X86: Introduce kvm_mmu_get_tdp_walk() for SEV-SNP use Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 29/37] KVM: X86: Define new RMP check related #NPF error bits Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 30/37] KVM: X86: update page-fault trace to log the 64-bit error code Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 31/37] KVM: SVM: Add support to handle GHCB GPA register VMGEXIT Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 32/37] KVM: SVM: Add support to handle MSR based Page State Change VMGEXIT Brijesh Singh
2021-05-10 17:30   ` Peter Gonda
2021-05-10 17:51     ` Brijesh Singh
2021-05-10 19:59       ` Peter Gonda
2021-05-10 20:50         ` Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 33/37] KVM: SVM: Add support to handle " Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 34/37] KVM: X86: Export the kvm_zap_gfn_range() for the SNP use Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 35/37] KVM: SVM: Add support to handle the RMP nested page fault Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 36/37] KVM: SVM: Provide support for SNP_GUEST_REQUEST NAE event Brijesh Singh
2021-05-10 18:57   ` Peter Gonda
2021-05-10 20:14     ` Brijesh Singh
2021-05-10 21:17       ` Sean Christopherson
2021-05-11 18:34         ` Brijesh Singh
2021-04-30 12:38 ` [PATCH Part2 RFC v2 37/37] KVM: SVM: Advertise the SEV-SNP feature support Brijesh Singh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9e3e4331-2933-7ae6-31d9-5fb73fce4353@amd.com \
    --to=brijesh.singh@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=jroedel@suse.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).