linux-kernel - Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function to KVM guest

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: <YweS9QRqaOgH7pNW@google.com>
Date:   Thu, 25 Aug 2022 15:19:17 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>
Cc:     Kai Huang <kai.huang@...el.com>, dave.hansen@...ux.intel.com,
        linux-sgx@...r.kernel.org, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, pbonzini@...hat.com,
        haitao.huang@...ux.intel.com
Subject: Re: [PATCH v2] x86/sgx: Allow exposing EDECCSSA user leaf function
 to KVM guest

On Thu, Aug 25, 2022, Jarkko Sakkinen wrote:
> Nit: shouldn't be this be x86/kvm?

Heh, no, because x86/kvm is the scope for Linux running as a KVM guest, i.e. for
changes to arch/x86/kernel/kvm.c.

But yeah, "KVM: x86:" or maybe even "KVM: VMX:" would be preferable given that all
of the meaningful changes are KVM specific.

> On Thu, Aug 18, 2022 at 02:38:29PM +1200, Kai Huang wrote:
> > The new Asynchronous Exit (AEX) notification mechanism (AEX-notify)
> > allows one enclave to receive a notification in the ERESUME after the
> > enclave exit due to an AEX.  EDECCSSA is a new SGX user leaf function
> > (ENCLU[EDECCSSA]) to facilitate the AEX notification handling.  The new
> > EDECCSSA is enumerated via CPUID(EAX=0x12,ECX=0x0):EAX[11].
> > 
> > Besides Allowing reporting the new AEX-notify attribute to KVM guests,
> > also allow reporting the new EDECCSSA user leaf function to KVM guests
> > so the guest can fully utilize the AEX-notify mechanism.
> > 
> > Similar to existing X86_FEATURE_SGX1 and X86_FEATURE_SGX2, introduce a
> > new scattered X86_FEATURE_SGX_EDECCSSA bit for the new EDECCSSA, and
> > report it in KVM's supported CPUIDs so the userspace hypervisor (i.e.
> > Qemu) can enable it for the guest.

Silly nit, but I'd prefer to leave off the "so the userspace hypervisor ... can
enable it for the guest".  Userspace doesn't actually need to wait for KVM enabling.
As noted below, KVM doesn't need to do anything extra, and KVM _can't_ prevent the
guest from using EDECCSSA.

> > Note there's no additional enabling work required to allow guest to use
> > the new EDECCSSA.  KVM is not able to trap ENCLU anyway.

And maybe call out that the KVM "enabling" is not strictly necessary?  And note
that there's a virtualization hole?  E.g.

  Note, no additional KVM enabling is required to allow the guest to use
  EDECCSSA, it's impossible to trap ENCLU (without completely preventing the
  guest from using SGX).  Advertise EDECCSSA as supported purely so that
  userspace doesn't need to special case EDECCSSA, i.e. doesn't need to
  manually check host CPUID.

  The inability to trap ENCLU also means that KVM can't prevent the guest
  from using EDECCSSA, but that virtualization hole is benign as far as KVM
  is concerned.  EDECCSSA is simply a fancy way to modify internal enclave
  state.

> > More background about how do AEX-notify and EDECCSSA work:
> > 
> > SGX maintains a Current State Save Area Frame (CSSA) for each enclave
> > thread.  When AEX happens, the enclave thread context is saved to the
> > CSSA and the CSSA is increased by 1.  For a normal ERESUME which doesn't
> > deliver AEX notification, it restores the saved thread context from the
> > previously saved SSA and decreases the CSSA.  If AEX-notify is enabled
> > for one enclave, the ERESUME acts differently.  Instead of restoring the
> > saved thread context and decreasing the CSSA, it acts like EENTER which
> > doesn't decrease the CSSA but establishes a clean slate thread context
> > using the CSSA for the enclave to handle the notification.  After some
> > handling, the enclave must discard the "new-established" SSA and switch
> > back to the previously saved SSA (upon AEX).  Otherwise, the enclave
> > will run out of SSA space upon further AEXs and eventually fail to run.
> > 
> > To solve this problem, the new EDECCSSA essentially decreases the CSSA.
> > It can be used by the enclave notification handler to switch back to the
> > previous saved SSA when needed, i.e. after it handles the notification.
> > 
> > Signed-off-by: Kai Huang <kai.huang@...el.com>
> > ---
> > 
> > Hi Dave,
> > 
> > This patch, along with your patch to expose AEX-notify attribute bit to
> > guest, have been tested that both AEX-notify and EDECCSSA work in the VM.
> > Feel free to merge this patch.

Dave, any objection to taking this through the KVM tree?