linux-kernel - Re: [PATCH v38 14/24] x86/sgx: Add SGX_IOC_ENCLAVE

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: <20200921181021.GA24481@linux.intel.com>
Date:   Mon, 21 Sep 2020 11:10:21 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>, x86@...nel.org,
        linux-sgx@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jethro Beekman <jethro@...tanix.com>,
        Haitao Huang <haitao.huang@...ux.intel.com>,
        Chunyang Hui <sanqian.hcy@...fin.com>,
        Jordan Hand <jorhand@...ux.microsoft.com>,
        Nathaniel McCallum <npmccallum@...hat.com>,
        Seth Moore <sethmo@...gle.com>,
        Darren Kenny <darren.kenny@...cle.com>,
        Suresh Siddha <suresh.b.siddha@...el.com>,
        akpm@...ux-foundation.org, andriy.shevchenko@...ux.intel.com,
        asapek@...gle.com, cedric.xing@...el.com, chenalexchen@...gle.com,
        conradparker@...gle.com, cyhanish@...gle.com,
        dave.hansen@...el.com, haitao.huang@...el.com,
        josh@...htriplett.org, kai.huang@...el.com, kai.svahn@...el.com,
        kmoy@...gle.com, ludloff@...gle.com, luto@...nel.org,
        nhorman@...hat.com, puiterwijk@...hat.com, rientjes@...gle.com,
        tglx@...utronix.de, yaozhangx@...gle.com
Subject: Re: [PATCH v38 14/24] x86/sgx: Add SGX_IOC_ENCLAVE_INIT

On Mon, Sep 21, 2020 at 07:35:14PM +0200, Borislav Petkov wrote:
> On Tue, Sep 15, 2020 at 02:28:32PM +0300, Jarkko Sakkinen wrote:
> > +static int sgx_einit(struct sgx_sigstruct *sigstruct, void *token,
> > +		     struct sgx_epc_page *secs, u64 *lepubkeyhash)
> > +{
> > +	int ret;
> > +
> > +	preempt_disable();
> > +	sgx_update_lepubkeyhash_msrs(lepubkeyhash, false);
> 
> So this will update the cached copies *and* the MSRs itself if what's
> cached is stale...
> 
> > +	ret = __einit(sigstruct, token, sgx_get_epc_addr(secs));
> > +	if (ret == SGX_INVALID_EINITTOKEN) {
> 
> ... so why would it return this error here?
> 
> Definition of this error says:
> 
>  * %SGX_INVALID_EINITTOKEN:     EINITTOKEN is invalid and enclave signer's
>  *                              public key does not match IA32_SGXLEPUBKEYHASH.
> 
> when you just updated them?!
> 
> > +		sgx_update_lepubkeyhash_msrs(lepubkeyhash, true);
> 
> So why force a second time?

The LE pubkey hash MSRs are special snowflakes.  They get reset to Intel's
default key on any loss of EPC, e.g. if the system does a suspend/resume
cycle.  The approach we took (obviously) is to assume the kernel's cache can
be stale at any given time.  The alternative would be to try and track loss
of EPC conditions and emulate the reset, but that's a bit dicey on bare
metal as any missed case would hose SGX, and in a VM it's theoretically
impossible to handle as a particularly unhelpful VMM could emulate loss of
EPC at will.

Yes, this need a big fat comment.

> > +		ret = __einit(sigstruct, token, sgx_get_epc_addr(secs));
> > +	}
> > +	preempt_enable();
> > +	return ret;
> > +}