linux-kernel - Re: [PATCH] [v2] x86/sgx: Allow enclaves to use Asynchrounous Exit Notification

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:   Wed, 20 Jul 2022 19:49:49 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     dave@...1.net, Jarkko Sakkinen <jarkko@...nel.org>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, Kai Huang <kai.huang@...el.com>,
        Haitao Huang <haitao.huang@...ux.intel.com>, x86@...nel.org,
        linux-sgx@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [v2] x86/sgx: Allow enclaves to use Asynchrounous Exit
 Notification

On Wed, Jul 20, 2022, Dave Hansen wrote:
> Changes from v1:
>  * Make sure SGX_ATTR_ASYNC_EXIT_NOTIFY is in the masks that are
>    used at bare-metal enclave initialization and that enumerates
>    available attributes to KVM guests.

Heh, I was wondering if KVM needed to be updated.

> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0c1ba6aa0765..96a73b5b4369 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1022,9 +1022,7 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
>  		 * userspace.  ATTRIBUTES.XFRM is not adjusted as userspace is
>  		 * expected to derive it from supported XCR0.
>  		 */
> -		entry->eax &= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT |
> -			      SGX_ATTR_PROVISIONKEY | SGX_ATTR_EINITTOKENKEY |
> -			      SGX_ATTR_KSS;
> +		entry->eax &= SGX_ATTR_PRIV_MASK | SGX_ATTR_UNPRIV_MASK;

It may seem like a maintenance burdern, and it is to some extent, but I think it's
better for KVM to have to explicitly "enable" each flag.  There is no guarantee
that a new feature will not require additional KVM enabling, i.e. we want the pain
of having to manually update KVM so that we get "feature X isn't virtualized"
complaints and not "I upgraded my kernel and my enclaves broke" bug reports.

I don't think it's likely that attribute-based features will require additional
enabling since there aren't any virtualization controls for the ENCLU side of
things (ENCLU is effectively disabled by blocking ENCLS[ECREATE]), but updating
KVM isn't particularly difficult so I'd rather be paranoid.