Re: [RFC PATCH v3 00/27] KVM SGX virtualization support

[Sean Christopherson <seanjc@google.com>]

On Thu, Feb 04, 2021, Kai Huang wrote:
> On Wed, 2021-02-03 at 16:28 -0800, Sean Christopherson wrote:
> > On Thu, Feb 04, 2021, Kai Huang wrote:
> > > On Wed, 2021-02-03 at 15:37 -0800, Dave Hansen wrote:
> > > > On 2/3/21 3:32 PM, Sean Christopherson wrote:
> > > > > > > > Yeah, special casing KVM is almost always the wrong thing to do.
> > > > > > > > Anything that KVM can do, other subsystems will do as well.
> > > > > > > Agreed.  Thwarting ioremap itself seems like the right way to go.
> > > > > > This sounds irrelevant to KVM SGX, thus I won't include it to KVM SGX series.
> > > > > I would say it's relevant, but a pre-existing bug.  Same net effect on what's
> > > > > needed for this series..
> > > > > 
> > > > > I say it's a pre-existing bug, because I'm pretty sure KVM can be coerced into
> > > > > accessing the EPC by handing KVM a memslot that's backed by an enclave that was
> > > > > created by host userspace (via /dev/sgx_enclave).
> > > > 
> > > > Dang, you beat me to it.  I was composing another email that said the
> > > > exact same thing.
> > > > 
> > > > I guess we need to take a closer look at the KVM fallout from this.
> > > > It's a few spots where it KVM knew it might be consuming garbage.  It
> > > > just get extra weird stinky garbage now.
> > > 
> > > I don't quite understand how KVM will need to access EPC memslot. It is *guest*, but
> > > not KVM, who can read EPC from non-enclave. And if I understand correctly, there will
> > > be no place for KVM to use kernel address of EPC to access it. To KVM, there's no
> > > difference, whether EPC backend is from /dev/sgx_enclave, or /dev/sgx_vepc. And we
> > > really cannot prevent guest from doing anything.
> > > 
> > > So how memremap() of EPC section is related to KVM SGX? For instance, the
> > > implementation of this series needs to be modified due to this?
> > 
> > See kvm_vcpu_map() -> __kvm_map_gfn(), which blindly uses memremap() when the
> > resulting pfn isn't a "valid" pfn.  KVM doesn't need access to an EPC memslot,
> > we're talking the case where a malicious userspace/guest hands KVM a GPA that
> > resolves to the EPC.  E.g. nested VM-Enter with the L1->L2 MSR bitmap pointing
> > at EPC.  L0 KVM will intercept VM-Enter and then read L1's bitmap to merge it's
> > desires with L0 KVM's requirements.  That read will hit the EPC, and thankfully
> > for KVM, return garbage.
> 
> Right. I missed __kvm_map_gfn(). 
> 
> I am not quite sure returning all ones can be treated as garbage, since one can means
> true for a boolean, or one bit in bitmap as you said. But since this only happens
> when guest/userspace is malicious, so causing misbehavior to the guest is fine?

Yes, it's fine.  It's really the guest causing misbehavior for itself.

> Do we see any security risk here?

Not with current CPUs, which drop writes and read all ones.  If future CPUs take
creatives liberties with the SDM, then we could have a problem, but that's why
Dave is trying to get stronger guarantees into the SDM.