linux-kernel - RE: [PATCH v3 2/2] x86/sgx: Implement EUPDATESVN and opportunistically call it during first EPC page alloc

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: <DM8PR11MB5750AB0E790096AFF9AFD3AFE7842@DM8PR11MB5750.namprd11.prod.outlook.com>
Date: Fri, 25 Apr 2025 06:52:53 +0000
From: "Reshetova, Elena" <elena.reshetova@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: "jarkko@...nel.org" <jarkko@...nel.org>, "Huang, Kai"
	<kai.huang@...el.com>, "Hansen, Dave" <dave.hansen@...el.com>,
	"linux-sgx@...r.kernel.org" <linux-sgx@...r.kernel.org>, "Scarlata, Vincent
 R" <vincent.r.scarlata@...el.com>, "x86@...nel.org" <x86@...nel.org>,
	"Annapurve, Vishal" <vannapurve@...gle.com>, "Cai, Chong"
	<chongc@...gle.com>, "Mallick, Asit K" <asit.k.mallick@...el.com>, "Aktas,
 Erdem" <erdemaktas@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "bondarn@...gle.com" <bondarn@...gle.com>,
	"dionnaglaze@...gle.com" <dionnaglaze@...gle.com>, "Raynor, Scott"
	<scott.raynor@...el.com>
Subject: RE: [PATCH v3 2/2] x86/sgx: Implement EUPDATESVN and
 opportunistically call it during first EPC page alloc

> On Thu, Apr 24, 2025, Elena Reshetova wrote:
> > > On Thu, Apr 24, 2025, Elena Reshetova wrote:
> > > +void sgx_dec_usage_count(void)
> > > +{
> > > +	if (atomic_dec_return(&sgx_usage_count))
> > > +		return;
> > > +
> > > +	guard(mutex)(&sgx_svn_lock);
> > > +
> > > +	if (atomic_read(&sgx_usage_count))
> > > +		return;
> > > +
> > > +	sgx_update_svn();
> >
> > Why do we want to try to execute this on release also?  I would think that
> > doing this in sgx_inc_usage_count() is enough.
> 
> I assume an actual SVN update takes some amount of time?

Yes, correct, it is not a fast action and can be even slower if we are
running out of entropy and have to retry.

> 
> If that's correct, then doing the work upon destroying the last enclave is
> desirable,
> as it's less likely to introduce delay that negatively affects userspace.
> Userspace
> generally won't care about a 10us delay when destroying a process, but a
> 10us delay
> to launch an enclave could be quite problematic, e.g. in the TDX use case
> where
> enclaves may be launched on-demand in response to a guest attestation
> request.

Ok, but in this case, you are hooking both: create and release. 
I guess your line of thinking is that in most of the cases it will be handled by
a release flow when destroying enclaves, but in cases we happen to need
to update a machine which doesn’t have a single enclave yet, the create flow
will be used. The problem is that if you look at the instruction definition, 
we won't save too much when executing this in release flow (Kai I think already pointed
this out):

IF (Other instruction is accessing EPC) THEN
 RFLAGS.ZF := 1
 RAX := SGX_LOCKFAIL;
 GOTO ERROR_EXIT;
FI
(* Verify EPC is ready *)
IF (the EPC contains any valid pages) THEN
 RFLAGS.ZF := 1;
 RAX := SGX_EPC_NOT_READY;
 GOTO ERROR_EXIT;
FI
(* Refresh paging key *)  
IF (NOT RDSEED(&TMP_KEY, 16)) THEN
 RFLAGS.ZF := 1;
 RAX := SGX_INSUFFICIENT_ENTROPY;
GOTO ERROR_EXIT;
FI
(* Commit *)
CR_BASE_KEY := TMP_KEY;
TMP_CPUSVN := CR_CPUSVN;
(* Update CPUSVN to current minimum patch even if locked *)
(* Determine if info status is needed *)

----------------------------
All above would be done anyhow on create even if we successfully
executed it on release previously (( And then finally we go into:

IF (TMP_CPUSVN = CR_CPUSVN) THEN
 RFLAGS.CF := 1;
 RAX := SGX_NO_UPDATE;
FI
ERROR_EXIT:

> 
> If the update time is tiny, then I agree that hooking release would probably do
> more harm than good.

So, it is not that the time is tiny, it is like we will do it twice, unnecessary
and potentially quite long in both cases (taking RDSEED step into account).

The reason why the instruction is defined this way is that it was not intended originally
to be inserted into some existing SGX flows, but was envisioned to be standalone cmd
for the host orchestrator to execute once it thinks system is ready. 

Best Regards,
Elena.